diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 496367ef26..4bc4230731 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/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 { - let mut changeset = 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( diff --git a/crates/chain/src/indexer.rs b/crates/chain/src/indexer.rs index 22e8398152..86b4dd4728 100644 --- a/crates/chain/src/indexer.rs +++ b/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(&mut self, graph: &TxGraph) -> 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 + } } diff --git a/crates/chain/src/indexer/keychain_txout.rs b/crates/chain/src/indexer/keychain_txout.rs index 7973a0254e..33b5c97270 100644 --- a/crates/chain/src/indexer/keychain_txout.rs +++ b/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 Indexer for KeychainTxOutIndex { 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(&mut self, graph: &TxGraph) -> 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 KeychainTxOutIndex { diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 96cafcb8ed..dd0c978930 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/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::>::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. ///