From 3b2ff0cc953204c9925ace8e2f0bbef409c63ad5 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Wed, 5 Jun 2024 16:39:50 +1000 Subject: [PATCH 01/14] Write failing test for keychain range querying --- .../chain/tests/test_keychain_txout_index.rs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index 55af741c..e1996bdc 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -777,3 +777,36 @@ fn test_only_highest_ord_keychain_is_returned() { Some((TestKeychain::External, 1)) ); } + +#[test] +fn when_querying_over_a_range_of_keychains_the_utxos_should_show_up() { + let mut indexer = KeychainTxOutIndex::::new(0); + let mut tx = common::new_tx(0); + + for (i, descriptor) in DESCRIPTORS.iter().enumerate() { + let descriptor = parse_descriptor(descriptor); + let _ = indexer.insert_descriptor(i, descriptor.clone()); + indexer.reveal_next_spk(&i); + tx.output.push(TxOut { + script_pubkey: descriptor.at_derivation_index(0).unwrap().script_pubkey(), + value: Amount::from_sat(10_000), + }); + } + + let _ = indexer.index_tx(&tx); + assert_eq!(indexer.outpoints().count(), DESCRIPTORS.len()); + + assert_eq!( + indexer.revealed_spks(0..DESCRIPTORS.len()).count(), + DESCRIPTORS.len() + ); + assert_eq!(indexer.revealed_spks(1..4).count(), 4 - 1); + assert_eq!( + indexer.net_value(&tx, 0..DESCRIPTORS.len()).to_sat(), + (10_000 * DESCRIPTORS.len()) as i64 + ); + assert_eq!( + indexer.net_value(&tx, 3..5).to_sat(), + (10_000 * (5 - 3)) as i64 + ); +} From bc2a8be97919f0d09b61438527bda24796bcec94 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 6 Jun 2024 10:17:55 +1000 Subject: [PATCH 02/14] refactor(keychain): Fix KeychainTxOutIndex range queries The underlying SpkTxOutIndex should not use DescriptorIds to index because this loses the ordering relationship of the spks so queries on subranges of keychains work. Along with that we enforce that there is a strict 1-to-1 relationship between descriptors and keychains. Violating this leads to an error in insert_descriptor now. In general I try to make the translation layer between the SpkTxOutIndex and the KeychainTxOutIndex thinner. Ergonomics of this will be improved in next commit. The test from the previous commit passes. --- clippy.toml | 1 + crates/chain/src/keychain/txout_index.rs | 639 +++++++++--------- crates/chain/src/spk_client.rs | 2 +- crates/chain/src/spk_txout_index.rs | 4 +- crates/chain/tests/test_indexed_tx_graph.rs | 28 +- .../chain/tests/test_keychain_txout_index.rs | 143 ++-- crates/wallet/src/descriptor/error.rs | 1 - crates/wallet/src/wallet/mod.rs | 50 +- .../example_bitcoind_rpc_polling/src/main.rs | 4 +- example-crates/example_cli/src/lib.rs | 6 +- example-crates/example_electrum/src/main.rs | 16 +- example-crates/example_esplora/src/main.rs | 14 +- 12 files changed, 441 insertions(+), 467 deletions(-) diff --git a/clippy.toml b/clippy.toml index 69478cea..fec36d9d 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,2 @@ msrv="1.63.0" +type-complexity-threshold = 275 diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 18690a7a..f4f736a4 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -5,7 +5,8 @@ use crate::{ spk_iter::BIP32_MAX_INDEX, DescriptorExt, DescriptorId, SpkIterator, SpkTxOutIndex, }; -use bitcoin::{hashes::Hash, Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid}; +use alloc::{borrow::ToOwned, vec::Vec}; +use bitcoin::{Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; use core::{ fmt::Debug, ops::{Bound, RangeBounds}, @@ -50,9 +51,17 @@ impl Append for ChangeSet { /// For each `last_revealed` in the given [`ChangeSet`]: /// If the keychain already exists, increase the index when the other's index > self's index. fn append(&mut self, other: Self) { - // We use `extend` instead of `BTreeMap::append` due to performance issues with `append`. - // Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420 - self.keychains_added.extend(other.keychains_added); + for (new_keychain, new_descriptor) in other.keychains_added { + if !self.keychains_added.contains_key(&new_keychain) + // FIXME: very inefficient + && self + .keychains_added + .values() + .all(|descriptor| descriptor != &new_descriptor) + { + self.keychains_added.insert(new_keychain, new_descriptor); + } + } // for `last_revealed`, entries of `other` will take precedence ONLY if it is greater than // what was originally in `self`. @@ -127,7 +136,7 @@ const DEFAULT_LOOKAHEAD: u32 = 25; /// /// # Change sets /// -/// Methods that can update the last revealed index or add keychains will return [`super::ChangeSet`] to report +/// Methods that can update the last revealed index or add keychains will return [`ChangeSet`] to report /// these changes. This can be persisted for future recovery. /// /// ## Synopsis @@ -206,22 +215,18 @@ const DEFAULT_LOOKAHEAD: u32 = 25; /// [`unused_spks`]: KeychainTxOutIndex::unused_spks #[derive(Clone, Debug)] pub struct KeychainTxOutIndex { - inner: SpkTxOutIndex<(DescriptorId, u32)>, + inner: SpkTxOutIndex<(K, u32)>, // keychain -> (descriptor, descriptor id) map - keychains_to_descriptors: BTreeMap)>, - // descriptor id -> keychain set - // Because different keychains can have the same descriptor, we rank keychains by `Ord` so that - // that the first keychain variant (according to `Ord`) has the highest rank. When associated - // data (such as spks, outpoints) are returned with a keychain, we return the highest-ranked - // keychain with it. - descriptor_ids_to_keychain_set: HashMap>, + keychains_to_descriptor_ids: BTreeMap, + // descriptor id -> keychain map + descriptor_ids_to_keychains: BTreeMap, // descriptor_id -> descriptor map // This is a "monotone" map, meaning that its size keeps growing, i.e., we never delete // descriptors from it. This is useful for revealing spks for descriptors that don't have // keychains associated. descriptor_ids_to_descriptors: BTreeMap>, // last revealed indexes - last_revealed: BTreeMap, + last_revealed: HashMap, // lookahead settings for each keychain lookahead: u32, } @@ -233,23 +238,26 @@ impl Default for KeychainTxOutIndex { } impl Indexer for KeychainTxOutIndex { - type ChangeSet = super::ChangeSet; + type ChangeSet = ChangeSet; fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet { - match self.inner.scan_txout(outpoint, txout).cloned() { - Some((descriptor_id, index)) => { - // We want to reveal spks for descriptors that aren't tracked by any keychain, and - // so we call reveal with descriptor_id - let (_, changeset) = self.reveal_to_target_with_id(descriptor_id, index) - .expect("descriptors are added in a monotone manner, there cannot be a descriptor id with no corresponding descriptor"); - changeset + let mut changeset = ChangeSet::default(); + if let Some((keychain, index)) = self.inner.scan_txout(outpoint, txout) { + let did = self + .keychains_to_descriptor_ids + .get(keychain) + .expect("invariant"); + if self.last_revealed.get(did) < Some(index) { + self.last_revealed.insert(*did, *index); + changeset.last_revealed.insert(*did, *index); + self.replenish_lookahead_did(*did); } - None => super::ChangeSet::default(), } + changeset } fn index_tx(&mut self, tx: &bitcoin::Transaction) -> Self::ChangeSet { - let mut changeset = super::ChangeSet::::default(); + let mut changeset = ChangeSet::::default(); for (op, txout) in tx.output.iter().enumerate() { changeset.append(self.index_txout(OutPoint::new(tx.compute_txid(), op as u32), txout)); } @@ -257,12 +265,12 @@ impl Indexer for KeychainTxOutIndex { } fn initial_changeset(&self) -> Self::ChangeSet { - super::ChangeSet { + ChangeSet { keychains_added: self .keychains() .map(|(k, v)| (k.clone(), v.clone())) .collect(), - last_revealed: self.last_revealed.clone(), + last_revealed: self.last_revealed.clone().into_iter().collect(), } } @@ -289,10 +297,10 @@ impl KeychainTxOutIndex { pub fn new(lookahead: u32) -> Self { Self { inner: SpkTxOutIndex::default(), - keychains_to_descriptors: BTreeMap::new(), - descriptor_ids_to_keychain_set: HashMap::new(), + keychains_to_descriptor_ids: BTreeMap::new(), descriptor_ids_to_descriptors: BTreeMap::new(), - last_revealed: BTreeMap::new(), + descriptor_ids_to_keychains: Default::default(), + last_revealed: Default::default(), lookahead, } } @@ -300,50 +308,30 @@ impl KeychainTxOutIndex { /// Methods that are *re-exposed* from the internal [`SpkTxOutIndex`]. impl KeychainTxOutIndex { - /// Get the highest-ranked keychain that is currently associated with the given `desc_id`. - fn keychain_of_desc_id(&self, desc_id: &DescriptorId) -> Option<&K> { - let keychains = self.descriptor_ids_to_keychain_set.get(desc_id)?; - keychains.iter().next() - } - /// Return a reference to the internal [`SpkTxOutIndex`]. /// /// **WARNING:** The internal index will contain lookahead spks. Refer to /// [struct-level docs](KeychainTxOutIndex) for more about `lookahead`. - pub fn inner(&self) -> &SpkTxOutIndex<(DescriptorId, u32)> { + pub fn inner(&self) -> &SpkTxOutIndex<(K, u32)> { &self.inner } /// Get the set of indexed outpoints, corresponding to tracked keychains. - pub fn outpoints(&self) -> impl DoubleEndedIterator + '_ { - self.inner - .outpoints() - .iter() - .filter_map(|((desc_id, index), op)| { - let keychain = self.keychain_of_desc_id(desc_id)?; - Some(((keychain.clone(), *index), *op)) - }) + pub fn outpoints(&self) -> &BTreeSet<((K, u32), OutPoint)> { + self.inner.outpoints() } /// Iterate over known txouts that spend to tracked script pubkeys. - pub fn txouts(&self) -> impl DoubleEndedIterator + '_ { - self.inner.txouts().filter_map(|((desc_id, i), op, txo)| { - let keychain = self.keychain_of_desc_id(desc_id)?; - Some((keychain.clone(), *i, op, txo)) - }) + pub fn txouts(&self) -> impl DoubleEndedIterator + '_ { + self.inner.txouts() } /// Finds all txouts on a transaction that has previously been scanned and indexed. pub fn txouts_in_tx( &self, txid: Txid, - ) -> impl DoubleEndedIterator { - self.inner - .txouts_in_tx(txid) - .filter_map(|((desc_id, i), op, txo)| { - let keychain = self.keychain_of_desc_id(desc_id)?; - Some((keychain.clone(), *i, op, txo)) - }) + ) -> impl DoubleEndedIterator { + self.inner.txouts_in_tx(txid) } /// Return the [`TxOut`] of `outpoint` if it has been indexed, and if it corresponds to a @@ -352,27 +340,22 @@ impl KeychainTxOutIndex { /// The associated keychain and keychain index of the txout's spk is also returned. /// /// This calls [`SpkTxOutIndex::txout`] internally. - pub fn txout(&self, outpoint: OutPoint) -> Option<(K, u32, &TxOut)> { - let ((descriptor_id, index), txo) = self.inner.txout(outpoint)?; - let keychain = self.keychain_of_desc_id(descriptor_id)?; - Some((keychain.clone(), *index, txo)) + pub fn txout(&self, outpoint: OutPoint) -> Option<(&(K, u32), &TxOut)> { + self.inner.txout(outpoint) } /// Return the script that exists under the given `keychain`'s `index`. /// /// This calls [`SpkTxOutIndex::spk_at_index`] internally. pub fn spk_at_index(&self, keychain: K, index: u32) -> Option<&Script> { - let descriptor_id = self.keychains_to_descriptors.get(&keychain)?.0; - self.inner.spk_at_index(&(descriptor_id, index)) + self.inner.spk_at_index(&(keychain.clone(), index)) } /// Returns the keychain and keychain index associated with the spk. /// /// This calls [`SpkTxOutIndex::index_of_spk`] internally. - pub fn index_of_spk(&self, script: &Script) -> Option<(K, u32)> { - let (desc_id, last_index) = self.inner.index_of_spk(script)?; - let keychain = self.keychain_of_desc_id(desc_id)?; - Some((keychain.clone(), *last_index)) + pub fn index_of_spk(&self, script: &Script) -> Option<&(K, u32)> { + self.inner.index_of_spk(script) } /// Returns whether the spk under the `keychain`'s `index` has been used. @@ -382,11 +365,7 @@ impl KeychainTxOutIndex { /// /// This calls [`SpkTxOutIndex::is_used`] internally. pub fn is_used(&self, keychain: K, index: u32) -> bool { - let descriptor_id = self.keychains_to_descriptors.get(&keychain).map(|k| k.0); - match descriptor_id { - Some(descriptor_id) => self.inner.is_used(&(descriptor_id, index)), - None => false, - } + self.inner.is_used(&(keychain, index)) } /// Marks the script pubkey at `index` as used even though the tracker hasn't seen an output @@ -406,11 +385,7 @@ impl KeychainTxOutIndex { /// /// [`unmark_used`]: Self::unmark_used pub fn mark_used(&mut self, keychain: K, index: u32) -> bool { - let descriptor_id = self.keychains_to_descriptors.get(&keychain).map(|k| k.0); - match descriptor_id { - Some(descriptor_id) => self.inner.mark_used(&(descriptor_id, index)), - None => false, - } + self.inner.mark_used(&(keychain, index)) } /// Undoes the effect of [`mark_used`]. Returns whether the `index` is inserted back into @@ -423,11 +398,7 @@ impl KeychainTxOutIndex { /// /// [`mark_used`]: Self::mark_used pub fn unmark_used(&mut self, keychain: K, index: u32) -> bool { - let descriptor_id = self.keychains_to_descriptors.get(&keychain).map(|k| k.0); - match descriptor_id { - Some(descriptor_id) => self.inner.unmark_used(&(descriptor_id, index)), - None => false, - } + self.inner.unmark_used(&(keychain, index)) } /// Computes the total value transfer effect `tx` has on the script pubkeys belonging to the @@ -462,9 +433,14 @@ impl KeychainTxOutIndex { &self, ) -> impl DoubleEndedIterator)> + ExactSizeIterator + '_ { - self.keychains_to_descriptors - .iter() - .map(|(k, (_, d))| (k, d)) + self.keychains_to_descriptor_ids.iter().map(|(k, did)| { + ( + k, + self.descriptor_ids_to_descriptors + .get(did) + .expect("invariant"), + ) + }) } /// Insert a descriptor with a keychain associated to it. @@ -472,56 +448,67 @@ impl KeychainTxOutIndex { /// Adding a descriptor means you will be able to derive new script pubkeys under it /// and the txout index will discover transaction outputs with those script pubkeys. /// - /// When trying to add a keychain that already existed under a different descriptor, or a descriptor - /// that already existed with a different keychain, the old keychain (or descriptor) will be - /// overwritten. + /// keychain <-> descriptor is a one-to-one mapping -- in `--release` this method will ignore calls that try to + /// associate a keychain with the descriptor of another keychain or to re-assign the keychain to + /// new descriptor. If `debug_assertions` are enabled then it will panic. pub fn insert_descriptor( &mut self, keychain: K, descriptor: Descriptor, - ) -> super::ChangeSet { - let mut changeset = super::ChangeSet::::default(); + ) -> Result, InsertDescriptorError> { + let mut changeset = ChangeSet::::default(); let desc_id = descriptor.descriptor_id(); - - let old_desc = self - .keychains_to_descriptors - .insert(keychain.clone(), (desc_id, descriptor.clone())); - - if let Some((old_desc_id, _)) = old_desc { - // nothing needs to be done if caller reinsterted the same descriptor under the same - // keychain - if old_desc_id == desc_id { - return changeset; + if !self.keychains_to_descriptor_ids.contains_key(&keychain) + && !self.descriptor_ids_to_keychains.contains_key(&desc_id) + { + self.descriptor_ids_to_descriptors + .insert(desc_id, descriptor.clone()); + self.keychains_to_descriptor_ids + .insert(keychain.clone(), desc_id); + self.descriptor_ids_to_keychains + .insert(desc_id, keychain.clone()); + self.replenish_lookahead(&keychain, self.lookahead); + changeset + .keychains_added + .insert(keychain.clone(), descriptor); + } else { + if let Some(existing_desc_id) = self.keychains_to_descriptor_ids.get(&keychain) { + let descriptor = self + .descriptor_ids_to_descriptors + .get(existing_desc_id) + .expect("invariant"); + if *existing_desc_id != desc_id { + return Err(InsertDescriptorError::KeychainAlreadyAssigned { + existing_assignment: descriptor.clone(), + keychain, + }); + } + } + + if let Some(existing_keychain) = self.descriptor_ids_to_keychains.get(&desc_id) { + let descriptor = self + .descriptor_ids_to_descriptors + .get(&desc_id) + .expect("invariant") + .clone(); + + if *existing_keychain != keychain { + return Err(InsertDescriptorError::DescriptorAlreadyAssigned { + existing_assignment: existing_keychain.clone(), + descriptor, + }); + } } - // we should remove old descriptor that is associated with this keychain as the index - // is designed to track one descriptor per keychain (however different keychains can - // share the same descriptor) - let _is_keychain_removed = self - .descriptor_ids_to_keychain_set - .get_mut(&old_desc_id) - .expect("we must have already inserted this descriptor") - .remove(&keychain); - debug_assert!(_is_keychain_removed); } - self.descriptor_ids_to_keychain_set - .entry(desc_id) - .or_default() - .insert(keychain.clone()); - self.descriptor_ids_to_descriptors - .insert(desc_id, descriptor.clone()); - self.replenish_lookahead(&keychain, self.lookahead); - - changeset - .keychains_added - .insert(keychain.clone(), descriptor); - changeset + Ok(changeset) } /// Gets the descriptor associated with the keychain. Returns `None` if the keychain doesn't /// have a descriptor associated with it. pub fn get_descriptor(&self, keychain: &K) -> Option<&Descriptor> { - self.keychains_to_descriptors.get(keychain).map(|(_, d)| d) + let did = self.keychains_to_descriptor_ids.get(keychain)?; + self.descriptor_ids_to_descriptors.get(did) } /// Get the lookahead setting. @@ -548,39 +535,44 @@ impl KeychainTxOutIndex { } } - fn replenish_lookahead(&mut self, keychain: &K, lookahead: u32) { - let descriptor_opt = self.keychains_to_descriptors.get(keychain).cloned(); - if let Some((descriptor_id, descriptor)) = descriptor_opt { - let next_store_index = self.next_store_index(descriptor_id); - let next_reveal_index = self.last_revealed.get(&descriptor_id).map_or(0, |v| *v + 1); + fn replenish_lookahead_did(&mut self, did: DescriptorId) { + if let Some(keychain) = self.descriptor_ids_to_keychains.get(&did).cloned() { + self.replenish_lookahead(&keychain, self.lookahead); + } + } + fn replenish_lookahead(&mut self, keychain: &K, lookahead: u32) { + if let Some(did) = self.keychains_to_descriptor_ids.get(keychain) { + let descriptor = self + .descriptor_ids_to_descriptors + .get(did) + .expect("invariant"); + let next_store_index = self + .inner + .all_spks() + .range(&(keychain.clone(), u32::MIN)..=&(keychain.clone(), u32::MAX)) + .last() + .map_or(0, |((_, index), _)| *index + 1); + let next_reveal_index = self.last_revealed.get(did).map_or(0, |v| *v + 1); for (new_index, new_spk) in SpkIterator::new_with_range( descriptor, next_store_index..next_reveal_index + lookahead, ) { - let _inserted = self.inner.insert_spk((descriptor_id, new_index), new_spk); + let _inserted = self + .inner + .insert_spk((keychain.clone(), new_index), new_spk); debug_assert!(_inserted, "replenish lookahead: must not have existing spk: keychain={:?}, lookahead={}, next_store_index={}, next_reveal_index={}", keychain, lookahead, next_store_index, next_reveal_index); } } } - fn next_store_index(&self, descriptor_id: DescriptorId) -> u32 { - self.inner() - .all_spks() - // This range is keeping only the spks with descriptor_id equal to - // `descriptor_id`. We don't use filter here as range is more optimized. - .range((descriptor_id, u32::MIN)..(descriptor_id, u32::MAX)) - .last() - .map_or(0, |((_, index), _)| *index + 1) - } - /// Get an unbounded spk iterator over a given `keychain`. Returns `None` if the provided /// keychain doesn't exist pub fn unbounded_spk_iter( &self, keychain: &K, ) -> Option>> { - let descriptor = self.keychains_to_descriptors.get(keychain)?.1.clone(); + let descriptor = self.get_descriptor(keychain)?.clone(); Some(SpkIterator::new(descriptor)) } @@ -588,9 +580,19 @@ impl KeychainTxOutIndex { pub fn all_unbounded_spk_iters( &self, ) -> BTreeMap>> { - self.keychains_to_descriptors + self.keychains_to_descriptor_ids .iter() - .map(|(k, (_, descriptor))| (k.clone(), SpkIterator::new(descriptor.clone()))) + .map(|(k, did)| { + ( + k.clone(), + SpkIterator::new( + self.descriptor_ids_to_descriptors + .get(did) + .expect("invariant") + .clone(), + ), + ) + }) .collect() } @@ -598,45 +600,65 @@ impl KeychainTxOutIndex { pub fn revealed_spks( &self, range: impl RangeBounds, - ) -> impl DoubleEndedIterator + Clone { - self.keychains_to_descriptors - .range(range) - .flat_map(|(_, (descriptor_id, _))| { - let start = Bound::Included((*descriptor_id, u32::MIN)); - let end = match self.last_revealed.get(descriptor_id) { - Some(last_revealed) => Bound::Included((*descriptor_id, *last_revealed)), - None => Bound::Excluded((*descriptor_id, u32::MIN)), - }; + ) -> impl Iterator { + let start = range.start_bound(); + let end = range.end_bound(); + let mut iter_last_revealed = self + .keychains_to_descriptor_ids + .range((start, end)) + .map(|(k, did)| (k, self.last_revealed.get(did).cloned())); + let mut iter_spks = self + .inner + .all_spks() + .range(self.map_to_inner_bounds((start, end))); + let mut current_keychain = iter_last_revealed.next(); + // The reason we need a tricky algorithm is because of the "lookahead" feature which means + // that some of the spks in the SpkTxoutIndex will not have been revealed yet. So we need to + // filter out those spks that are above the last_revealed for that keychain. To do this we + // iterate through the last_revealed for each keychain and the spks for each keychain in + // tandem. This minimizes BTreeMap queries. + core::iter::from_fn(move || loop { + let (path, spk) = iter_spks.next()?; + let (keychain, index) = path; + // We need to find the last revealed that matches the current spk we are considering so + // we skip ahead. + while current_keychain?.0 < keychain { + current_keychain = iter_last_revealed.next(); + } + let (current_keychain, last_revealed) = current_keychain?; - self.inner - .all_spks() - .range((start, end)) - .map(|((descriptor_id, i), spk)| { - ( - self.keychain_of_desc_id(descriptor_id) - .expect("must have keychain"), - *i, - spk.as_script(), - ) - }) - }) + if current_keychain == keychain && Some(*index) <= last_revealed { + break Some((path, spk.as_script())); + } + }) } - /// Iterate over revealed spks of the given `keychain`. + /// Iterate over revealed spks of the given `keychain` with ascending indices. + /// + /// This is a double ended iterator so you can easily reverse it to get an iterator where + /// the script pubkeys that were most recently revealed are first. pub fn revealed_keychain_spks<'a>( &'a self, keychain: &'a K, ) -> impl DoubleEndedIterator + 'a { - self.revealed_spks(keychain..=keychain) - .map(|(_, i, spk)| (i, spk)) + let end = self + .last_revealed_index(keychain) + .map(|v| v + 1) + .unwrap_or(0); + self.inner + .all_spks() + .range((keychain.clone(), 0)..(keychain.clone(), end)) + .map(|((_, index), spk)| (*index, spk.as_script())) } /// Iterate over revealed, but unused, spks of all keychains. - pub fn unused_spks(&self) -> impl DoubleEndedIterator + Clone { - self.keychains_to_descriptors.keys().flat_map(|keychain| { - self.unused_keychain_spks(keychain) - .map(|(i, spk)| (keychain.clone(), i, spk)) - }) + pub fn unused_spks(&self) -> impl DoubleEndedIterator + Clone { + self.keychains_to_descriptor_ids + .keys() + .flat_map(|keychain| { + self.unused_keychain_spks(keychain) + .map(|(i, spk)| ((keychain.clone(), i), spk)) + }) } /// Iterate over revealed, but unused, spks of the given `keychain`. @@ -645,17 +667,13 @@ impl KeychainTxOutIndex { &self, keychain: &K, ) -> impl DoubleEndedIterator + Clone { - let desc_id = self - .keychains_to_descriptors - .get(keychain) - .map(|(desc_id, _)| *desc_id) - // We use a dummy desc id if we can't find the real one in our map. In this way, - // if this method was to be called with a non-existent keychain, we would return an - // empty iterator - .unwrap_or_else(|| DescriptorId::from_byte_array([0; 32])); - let next_i = self.last_revealed.get(&desc_id).map_or(0, |&i| i + 1); + let end = match self.keychains_to_descriptor_ids.get(keychain) { + Some(did) => self.last_revealed.get(did).map(|v| *v + 1).unwrap_or(0), + None => 0, + }; + self.inner - .unused_spks((desc_id, u32::MIN)..(desc_id, next_i)) + .unused_spks((keychain.clone(), 0)..(keychain.clone(), end)) .map(|((_, i), spk)| (*i, spk)) } @@ -672,8 +690,12 @@ impl KeychainTxOutIndex { /// /// Returns None if the provided `keychain` doesn't exist. pub fn next_index(&self, keychain: &K) -> Option<(u32, bool)> { - let (descriptor_id, descriptor) = self.keychains_to_descriptors.get(keychain)?; - let last_index = self.last_revealed.get(descriptor_id).cloned(); + let did = self.keychains_to_descriptor_ids.get(keychain)?; + let last_index = self.last_revealed.get(did).cloned(); + let descriptor = self + .descriptor_ids_to_descriptors + .get(did) + .expect("invariant"); // we can only get the next index if the wildcard exists. let has_wildcard = descriptor.has_wildcard(); @@ -700,7 +722,7 @@ impl KeychainTxOutIndex { self.last_revealed .iter() .filter_map(|(desc_id, index)| { - let keychain = self.keychain_of_desc_id(desc_id)?; + let keychain = self.descriptor_ids_to_keychains.get(desc_id)?; Some((keychain.clone(), *index)) }) .collect() @@ -709,91 +731,21 @@ impl KeychainTxOutIndex { /// Get the last derivation index revealed for `keychain`. Returns None if the keychain doesn't /// exist, or if the keychain doesn't have any revealed scripts. pub fn last_revealed_index(&self, keychain: &K) -> Option { - let descriptor_id = self.keychains_to_descriptors.get(keychain)?.0; - self.last_revealed.get(&descriptor_id).cloned() + let descriptor_id = self.keychains_to_descriptor_ids.get(keychain)?; + self.last_revealed.get(descriptor_id).cloned() } /// Convenience method to call [`Self::reveal_to_target`] on multiple keychains. - pub fn reveal_to_target_multi( - &mut self, - keychains: &BTreeMap, - ) -> ( - BTreeMap>>, - super::ChangeSet, - ) { - let mut changeset = super::ChangeSet::default(); - let mut spks = BTreeMap::new(); + pub fn reveal_to_target_multi(&mut self, keychains: &BTreeMap) -> ChangeSet { + let mut changeset = ChangeSet::default(); for (keychain, &index) in keychains { - if let Some((new_spks, new_changeset)) = self.reveal_to_target(keychain, index) { - if !new_changeset.is_empty() { - spks.insert(keychain.clone(), new_spks); - changeset.append(new_changeset.clone()); - } + if let Some((_, new_changeset)) = self.reveal_to_target(keychain, index) { + changeset.append(new_changeset); } } - (spks, changeset) - } - - /// Convenience method to call `reveal_to_target` with a descriptor_id instead of a keychain. - /// This is useful for revealing spks of descriptors for which we don't have a keychain - /// tracked. - /// Refer to the `reveal_to_target` documentation for more. - /// - /// Returns None if the provided `descriptor_id` doesn't correspond to a tracked descriptor. - fn reveal_to_target_with_id( - &mut self, - descriptor_id: DescriptorId, - target_index: u32, - ) -> Option<( - SpkIterator>, - super::ChangeSet, - )> { - let descriptor = self - .descriptor_ids_to_descriptors - .get(&descriptor_id)? - .clone(); - let has_wildcard = descriptor.has_wildcard(); - - let target_index = if has_wildcard { target_index } else { 0 }; - let next_reveal_index = self - .last_revealed - .get(&descriptor_id) - .map_or(0, |index| *index + 1); - - debug_assert!(next_reveal_index + self.lookahead >= self.next_store_index(descriptor_id)); - - // If the target_index is already revealed, we are done - if next_reveal_index > target_index { - return Some(( - SpkIterator::new_with_range(descriptor, next_reveal_index..next_reveal_index), - super::ChangeSet::default(), - )); - } - - // We range over the indexes that are not stored and insert their spks in the index. - // Indexes from next_reveal_index to next_reveal_index + lookahead are already stored (due - // to lookahead), so we only range from next_reveal_index + lookahead to target + lookahead - let range = next_reveal_index + self.lookahead..=target_index + self.lookahead; - for (new_index, new_spk) in SpkIterator::new_with_range(descriptor.clone(), range) { - let _inserted = self.inner.insert_spk((descriptor_id, new_index), new_spk); - debug_assert!(_inserted, "must not have existing spk"); - debug_assert!( - has_wildcard || new_index == 0, - "non-wildcard descriptors must not iterate past index 0" - ); - } - - let _old_index = self.last_revealed.insert(descriptor_id, target_index); - debug_assert!(_old_index < Some(target_index)); - Some(( - SpkIterator::new_with_range(descriptor, next_reveal_index..target_index + 1), - super::ChangeSet { - keychains_added: BTreeMap::new(), - last_revealed: core::iter::once((descriptor_id, target_index)).collect(), - }, - )) + changeset } /// Reveals script pubkeys of the `keychain`'s descriptor **up to and including** the @@ -803,8 +755,8 @@ impl KeychainTxOutIndex { /// the `target_index` is in the hardened index range), this method will make a best-effort and /// reveal up to the last possible index. /// - /// This returns an iterator of newly revealed indices (alongside their scripts) and a - /// [`super::ChangeSet`], which reports updates to the latest revealed index. If no new script + /// This returns list of newly revealed indices (alongside their scripts) and a + /// [`ChangeSet`], which reports updates to the latest revealed index. If no new script /// pubkeys are revealed, then both of these will be empty. /// /// Returns None if the provided `keychain` doesn't exist. @@ -812,40 +764,57 @@ impl KeychainTxOutIndex { &mut self, keychain: &K, target_index: u32, - ) -> Option<( - SpkIterator>, - super::ChangeSet, - )> { - let descriptor_id = self.keychains_to_descriptors.get(keychain)?.0; - self.reveal_to_target_with_id(descriptor_id, target_index) + ) -> Option<(Vec<(u32, ScriptBuf)>, ChangeSet)> { + let mut changeset = ChangeSet::default(); + let mut spks: Vec<(u32, ScriptBuf)> = vec![]; + while let Some((i, new)) = self.next_index(keychain) { + if !new || i > target_index { + break; + } + match self.reveal_next_spk(keychain) { + Some(((i, spk), change)) => { + spks.push((i, spk.to_owned())); + changeset.append(change); + } + None => break, + } + } + + Some((spks, changeset)) } /// Attempts to reveal the next script pubkey for `keychain`. /// /// Returns the derivation index of the revealed script pubkey, the revealed script pubkey and a - /// [`super::ChangeSet`] which represents changes in the last revealed index (if any). + /// [`ChangeSet`] which represents changes in the last revealed index (if any). /// Returns None if the provided keychain doesn't exist. /// /// When a new script cannot be revealed, we return the last revealed script and an empty - /// [`super::ChangeSet`]. There are two scenarios when a new script pubkey cannot be derived: + /// [`ChangeSet`]. There are two scenarios when a new script pubkey cannot be derived: /// /// 1. The descriptor has no wildcard and already has one script revealed. /// 2. The descriptor has already revealed scripts up to the numeric bound. /// 3. There is no descriptor associated with the given keychain. - pub fn reveal_next_spk( - &mut self, - keychain: &K, - ) -> Option<((u32, &Script), super::ChangeSet)> { - let descriptor_id = self.keychains_to_descriptors.get(keychain)?.0; - let (next_index, _) = self.next_index(keychain).expect("We know keychain exists"); - let changeset = self - .reveal_to_target(keychain, next_index) - .expect("We know keychain exists") - .1; + pub fn reveal_next_spk(&mut self, keychain: &K) -> Option<((u32, &Script), ChangeSet)> { + let (next_index, new) = self.next_index(keychain)?; + let mut changeset = ChangeSet::default(); + + if new { + let did = self.keychains_to_descriptor_ids.get(keychain)?; + let descriptor = self.descriptor_ids_to_descriptors.get(did)?; + let spk = descriptor + .at_derivation_index(next_index) + .expect("already checked index is not too high") + .script_pubkey(); + let _ = self.inner.insert_spk((keychain.clone(), next_index), spk); + self.last_revealed.insert(*did, next_index); + changeset.last_revealed.insert(*did, next_index); + self.replenish_lookahead(keychain, self.lookahead); + } let script = self .inner - .spk_at_index(&(descriptor_id, next_index)) - .expect("script must already be stored"); + .spk_at_index(&(keychain.clone(), next_index)) + .expect("we just inserted it"); Some(((next_index, script), changeset)) } @@ -859,10 +828,7 @@ impl KeychainTxOutIndex { /// returned. /// /// Returns None if the provided keychain doesn't exist. - pub fn next_unused_spk( - &mut self, - keychain: &K, - ) -> Option<((u32, &Script), super::ChangeSet)> { + pub fn next_unused_spk(&mut self, keychain: &K) -> Option<((u32, &Script), ChangeSet)> { let need_new = self.unused_keychain_spks(keychain).next().is_none(); // this rather strange branch is needed because of some lifetime issues if need_new { @@ -872,7 +838,7 @@ impl KeychainTxOutIndex { self.unused_keychain_spks(keychain) .next() .expect("we already know next exists"), - super::ChangeSet::default(), + ChangeSet::default(), )) } } @@ -884,43 +850,26 @@ impl KeychainTxOutIndex { keychain: &'a K, ) -> impl DoubleEndedIterator + 'a { self.keychain_outpoints_in_range(keychain..=keychain) - .map(move |(_, i, op)| (i, op)) + .map(|((_, i), op)| (*i, op)) } /// Iterate over [`OutPoint`]s that have script pubkeys derived from keychains in `range`. pub fn keychain_outpoints_in_range<'a>( &'a self, range: impl RangeBounds + 'a, - ) -> impl DoubleEndedIterator + 'a { - let bounds = self.map_to_inner_bounds(range); - self.inner - .outputs_in_range(bounds) - .map(move |((desc_id, i), op)| { - let keychain = self - .keychain_of_desc_id(desc_id) - .expect("keychain must exist"); - (keychain, *i, op) - }) + ) -> impl DoubleEndedIterator + 'a { + self.inner.outputs_in_range(self.map_to_inner_bounds(range)) } - fn map_to_inner_bounds( - &self, - bound: impl RangeBounds, - ) -> impl RangeBounds<(DescriptorId, u32)> { - let get_desc_id = |keychain| { - self.keychains_to_descriptors - .get(keychain) - .map(|(desc_id, _)| *desc_id) - .unwrap_or_else(|| DescriptorId::from_byte_array([0; 32])) - }; + fn map_to_inner_bounds(&self, bound: impl RangeBounds) -> impl RangeBounds<(K, u32)> { let start = match bound.start_bound() { - Bound::Included(keychain) => Bound::Included((get_desc_id(keychain), u32::MIN)), - Bound::Excluded(keychain) => Bound::Excluded((get_desc_id(keychain), u32::MAX)), + Bound::Included(keychain) => Bound::Included((keychain.clone(), u32::MIN)), + Bound::Excluded(keychain) => Bound::Excluded((keychain.clone(), u32::MAX)), Bound::Unbounded => Bound::Unbounded, }; let end = match bound.end_bound() { - Bound::Included(keychain) => Bound::Included((get_desc_id(keychain), u32::MAX)), - Bound::Excluded(keychain) => Bound::Excluded((get_desc_id(keychain), u32::MIN)), + Bound::Included(keychain) => Bound::Included((keychain.clone(), u32::MAX)), + Bound::Excluded(keychain) => Bound::Excluded((keychain.clone(), u32::MIN)), Bound::Unbounded => Bound::Unbounded, }; @@ -936,7 +885,7 @@ impl KeychainTxOutIndex { /// Returns the highest derivation index of each keychain that [`KeychainTxOutIndex`] has found /// a [`TxOut`] with it's script pubkey. pub fn last_used_indices(&self) -> BTreeMap { - self.keychains_to_descriptors + self.keychains_to_descriptor_ids .iter() .filter_map(|(keychain, _)| { self.last_used_index(keychain) @@ -951,7 +900,7 @@ impl KeychainTxOutIndex { /// - Adds new descriptors introduced /// - If a descriptor is introduced for a keychain that already had a descriptor, overwrites /// the old descriptor - pub fn apply_changeset(&mut self, changeset: super::ChangeSet) { + pub fn apply_changeset(&mut self, changeset: ChangeSet) { let ChangeSet { keychains_added, last_revealed, @@ -959,13 +908,61 @@ impl KeychainTxOutIndex { for (keychain, descriptor) in keychains_added { let _ = self.insert_descriptor(keychain, descriptor); } - let last_revealed = last_revealed - .into_iter() - .filter_map(|(desc_id, index)| { - let keychain = self.keychain_of_desc_id(&desc_id)?; - Some((keychain.clone(), index)) - }) - .collect(); - let _ = self.reveal_to_target_multi(&last_revealed); + + for (&desc_id, &index) in &last_revealed { + let v = self.last_revealed.entry(desc_id).or_default(); + *v = index.max(*v); + } + + for did in last_revealed.keys() { + self.replenish_lookahead_did(*did); + } } } + +#[derive(Clone, Debug, PartialEq)] +/// Error returned from [`KeychainTxOutIndex::insert_descriptor`] +pub enum InsertDescriptorError { + /// The descriptor has already been assigned to a keychain so you can't assign it to another + DescriptorAlreadyAssigned { + /// The descriptor you have attempted to reassign + descriptor: Descriptor, + /// The keychain that the descriptor is already assigned to + existing_assignment: K, + }, + /// The keychain is already assigned to a descriptor so you can't reassign it + KeychainAlreadyAssigned { + /// The keychain that you have attempted to reassign + keychain: K, + /// The descriptor that the keychain is already assigned to + existing_assignment: Descriptor, + }, +} + +impl core::fmt::Display for InsertDescriptorError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + InsertDescriptorError::DescriptorAlreadyAssigned { + existing_assignment: existing, + descriptor, + } => { + write!( + f, + "attempt to re-assign descriptor {descriptor:?} already assigned to {existing:?}" + ) + } + InsertDescriptorError::KeychainAlreadyAssigned { + existing_assignment: existing, + keychain, + } => { + write!( + f, + "attempt to re-assign keychain {keychain:?} already assigned to {existing:?}" + ) + } + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InsertDescriptorError {} diff --git a/crates/chain/src/spk_client.rs b/crates/chain/src/spk_client.rs index fdc3be35..d65ecdba 100644 --- a/crates/chain/src/spk_client.rs +++ b/crates/chain/src/spk_client.rs @@ -166,7 +166,7 @@ impl SyncRequest { self.chain_spks( index .revealed_spks(spk_range) - .map(|(_, _, spk)| spk.to_owned()) + .map(|(_, spk)| spk.to_owned()) .collect::>(), ) } diff --git a/crates/chain/src/spk_txout_index.rs b/crates/chain/src/spk_txout_index.rs index a724ccdb..f664af89 100644 --- a/crates/chain/src/spk_txout_index.rs +++ b/crates/chain/src/spk_txout_index.rs @@ -52,7 +52,7 @@ impl Default for SpkTxOutIndex { } } -impl Indexer for SpkTxOutIndex { +impl Indexer for SpkTxOutIndex { type ChangeSet = (); fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet { @@ -76,7 +76,7 @@ impl Indexer for SpkTxOutIndex { } } -impl SpkTxOutIndex { +impl SpkTxOutIndex { /// Scans a transaction's outputs for matching script pubkeys. /// /// Typically, this is used in two situations: diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 43085cc2..f386f0cf 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::{ indexed_tx_graph::{self, IndexedTxGraph}, keychain::{self, Balance, KeychainTxOutIndex}, local_chain::LocalChain, - tx_graph, ChainPosition, ConfirmationHeightAnchor, DescriptorExt, + tx_graph, Append, ChainPosition, ConfirmationHeightAnchor, DescriptorExt, }; use bitcoin::{ secp256k1::Secp256k1, Amount, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut, @@ -140,8 +140,16 @@ fn test_list_owned_txouts() { KeychainTxOutIndex::new(10), ); - let _ = graph.index.insert_descriptor("keychain_1".into(), desc_1); - let _ = graph.index.insert_descriptor("keychain_2".into(), desc_2); + assert!(!graph + .index + .insert_descriptor("keychain_1".into(), desc_1) + .unwrap() + .is_empty()); + assert!(!graph + .index + .insert_descriptor("keychain_2".into(), desc_2) + .unwrap() + .is_empty()); // Get trusted and untrusted addresses @@ -257,18 +265,26 @@ fn test_list_owned_txouts() { .unwrap_or_else(|| panic!("block must exist at {}", height)); let txouts = graph .graph() - .filter_chain_txouts(&local_chain, chain_tip, graph.index.outpoints()) + .filter_chain_txouts( + &local_chain, + chain_tip, + graph.index.outpoints().iter().cloned(), + ) .collect::>(); let utxos = graph .graph() - .filter_chain_unspents(&local_chain, chain_tip, graph.index.outpoints()) + .filter_chain_unspents( + &local_chain, + chain_tip, + graph.index.outpoints().iter().cloned(), + ) .collect::>(); let balance = graph.graph().balance( &local_chain, chain_tip, - graph.index.outpoints(), + graph.index.outpoints().iter().cloned(), |_, spk: &Script| trusted_spks.contains(&spk.to_owned()), ); diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index e1996bdc..b2bdba5a 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -98,7 +98,7 @@ fn append_changesets_check_last_revealed() { } #[test] -fn test_apply_changeset_with_different_descriptors_to_same_keychain() { +fn when_apply_contradictory_changesets_they_are_ignored() { let external_descriptor = parse_descriptor(DESCRIPTORS[0]); let internal_descriptor = parse_descriptor(DESCRIPTORS[1]); let mut txout_index = @@ -120,7 +120,7 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() { assert_eq!( txout_index.keychains().collect::>(), vec![ - (&TestKeychain::External, &internal_descriptor), + (&TestKeychain::External, &external_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![ - (&TestKeychain::External, &internal_descriptor), - (&TestKeychain::Internal, &external_descriptor) + (&TestKeychain::External, &external_descriptor), + (&TestKeychain::Internal, &internal_descriptor) ] ); } @@ -156,7 +156,7 @@ fn test_set_all_derivation_indices() { ] .into(); assert_eq!( - txout_index.reveal_to_target_multi(&derive_to).1, + txout_index.reveal_to_target_multi(&derive_to), ChangeSet { keychains_added: BTreeMap::new(), last_revealed: last_revealed.clone() @@ -164,7 +164,7 @@ fn test_set_all_derivation_indices() { ); assert_eq!(txout_index.last_revealed_indices(), derive_to); assert_eq!( - txout_index.reveal_to_target_multi(&derive_to).1, + txout_index.reveal_to_target_multi(&derive_to), keychain::ChangeSet::default(), "no changes if we set to the same thing" ); @@ -190,7 +190,7 @@ fn test_lookahead() { .reveal_to_target(&TestKeychain::External, index) .unwrap(); assert_eq!( - revealed_spks.collect::>(), + revealed_spks, vec![(index, spk_at_index(&external_descriptor, index))], ); assert_eq!( @@ -241,7 +241,7 @@ fn test_lookahead() { .reveal_to_target(&TestKeychain::Internal, 24) .unwrap(); assert_eq!( - revealed_spks.collect::>(), + revealed_spks, (0..=24) .map(|index| (index, spk_at_index(&internal_descriptor, index))) .collect::>(), @@ -511,7 +511,7 @@ fn test_non_wildcard_derivations() { let (revealed_spks, revealed_changeset) = txout_index .reveal_to_target(&TestKeychain::External, 200) .unwrap(); - assert_eq!(revealed_spks.count(), 0); + assert_eq!(revealed_spks.len(), 0); assert!(revealed_changeset.is_empty()); // we check that spks_of_keychain returns a SpkIterator with just one element @@ -591,19 +591,17 @@ fn lookahead_to_target() { let keychain_test_cases = [ ( - external_descriptor.descriptor_id(), TestKeychain::External, t.external_last_revealed, t.external_target, ), ( - internal_descriptor.descriptor_id(), TestKeychain::Internal, t.internal_last_revealed, t.internal_target, ), ]; - for (descriptor_id, keychain, last_revealed, target) in keychain_test_cases { + for (keychain, last_revealed, target) in keychain_test_cases { if let Some(target) = target { let original_last_stored_index = match last_revealed { Some(last_revealed) => Some(last_revealed + t.lookahead), @@ -619,10 +617,10 @@ fn lookahead_to_target() { let keys = index .inner() .all_spks() - .range((descriptor_id, 0)..=(descriptor_id, u32::MAX)) - .map(|(k, _)| *k) + .range((keychain.clone(), 0)..=(keychain.clone(), u32::MAX)) + .map(|(k, _)| k.clone()) .collect::>(); - let exp_keys = core::iter::repeat(descriptor_id) + let exp_keys = core::iter::repeat(keychain) .zip(0_u32..=exp_last_stored_index) .collect::>(); assert_eq!(keys, exp_keys); @@ -631,50 +629,6 @@ fn lookahead_to_target() { } } -/// `::index_txout` should still index txouts with spks derived from descriptors without keychains. -/// This includes properly refilling the lookahead for said descriptors. -#[test] -fn index_txout_after_changing_descriptor_under_keychain() { - let secp = bdk_chain::bitcoin::secp256k1::Secp256k1::signing_only(); - let (desc_a, _) = Descriptor::::parse_descriptor(&secp, DESCRIPTORS[0]) - .expect("descriptor 0 must be valid"); - let (desc_b, _) = Descriptor::::parse_descriptor(&secp, DESCRIPTORS[1]) - .expect("descriptor 1 must be valid"); - let desc_id_a = desc_a.descriptor_id(); - - let mut txout_index = bdk_chain::keychain::KeychainTxOutIndex::<()>::new(10); - - // 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()); - - // Loop through spks in intervals of `lookahead` to create outputs with. We should always be - // able to index these outputs if `lookahead` is respected. - let spk_indices = [9, 19, 29, 39]; - for i in spk_indices { - let spk_at_index = desc_a - .at_derivation_index(i) - .expect("must derive") - .script_pubkey(); - let index_changeset = txout_index.index_txout( - // Use spk derivation index as vout as we just want an unique outpoint. - OutPoint::new(h!("mock_tx"), i as _), - &TxOut { - value: Amount::from_sat(10_000), - script_pubkey: spk_at_index, - }, - ); - assert_eq!( - index_changeset, - bdk_chain::keychain::ChangeSet { - keychains_added: BTreeMap::default(), - last_revealed: [(desc_id_a, i)].into(), - }, - "must always increase last active if impl respects lookahead" - ); - } -} - #[test] fn insert_descriptor_no_change() { let secp = Secp256k1::signing_only(); @@ -683,19 +637,20 @@ fn insert_descriptor_no_change() { let mut txout_index = KeychainTxOutIndex::<()>::default(); assert_eq!( txout_index.insert_descriptor((), desc.clone()), - keychain::ChangeSet { + Ok(keychain::ChangeSet { keychains_added: [((), desc.clone())].into(), last_revealed: Default::default() - }, + }), ); assert_eq!( txout_index.insert_descriptor((), desc.clone()), - keychain::ChangeSet::default(), + Ok(keychain::ChangeSet::default()), "inserting the same descriptor for keychain should return an empty changeset", ); } #[test] +#[cfg(not(debug_assertions))] fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() { let desc = parse_descriptor(DESCRIPTORS[0]); let changesets: &[ChangeSet] = &[ @@ -743,39 +698,25 @@ fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() { ); } -// When the same descriptor is associated with various keychains, -// index methods only return the highest keychain by Ord #[test] -fn test_only_highest_ord_keychain_is_returned() { +fn assigning_same_descriptor_to_multiple_keychains_should_error() { let desc = parse_descriptor(DESCRIPTORS[0]); - let mut indexer = KeychainTxOutIndex::::new(0); let _ = indexer.insert_descriptor(TestKeychain::Internal, desc.clone()); - let _ = indexer.insert_descriptor(TestKeychain::External, desc); + assert!(indexer + .insert_descriptor(TestKeychain::External, desc) + .is_err()) +} - // 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(); - - // index_of_spk will always return External - assert_eq!( - indexer.index_of_spk(&spk0), - Some((TestKeychain::External, 0)) - ); - assert_eq!( - indexer.index_of_spk(&spk1), - Some((TestKeychain::External, 1)) - ); +#[test] +fn reassigning_keychain_to_a_new_descriptor_should_error() { + let desc1 = parse_descriptor(DESCRIPTORS[0]); + let desc2 = parse_descriptor(DESCRIPTORS[1]); + let mut indexer = KeychainTxOutIndex::::new(0); + let _ = indexer.insert_descriptor(TestKeychain::Internal, desc1); + assert!(indexer + .insert_descriptor(TestKeychain::Internal, desc2) + .is_err()); } #[test] @@ -786,27 +727,29 @@ fn when_querying_over_a_range_of_keychains_the_utxos_should_show_up() { for (i, descriptor) in DESCRIPTORS.iter().enumerate() { let descriptor = parse_descriptor(descriptor); let _ = indexer.insert_descriptor(i, descriptor.clone()); - indexer.reveal_next_spk(&i); + if i != 4 { + // skip one in the middle to see if uncovers any bugs + indexer.reveal_next_spk(&i); + } tx.output.push(TxOut { script_pubkey: descriptor.at_derivation_index(0).unwrap().script_pubkey(), value: Amount::from_sat(10_000), }); } - let _ = indexer.index_tx(&tx); - assert_eq!(indexer.outpoints().count(), DESCRIPTORS.len()); + let n_spks = DESCRIPTORS.len() - /*we skipped one*/ 1; - assert_eq!( - indexer.revealed_spks(0..DESCRIPTORS.len()).count(), - DESCRIPTORS.len() - ); + let _ = indexer.index_tx(&tx); + assert_eq!(indexer.outpoints().len(), n_spks); + + assert_eq!(indexer.revealed_spks(0..DESCRIPTORS.len()).count(), n_spks); assert_eq!(indexer.revealed_spks(1..4).count(), 4 - 1); assert_eq!( indexer.net_value(&tx, 0..DESCRIPTORS.len()).to_sat(), - (10_000 * DESCRIPTORS.len()) as i64 + (10_000 * n_spks) as i64 ); assert_eq!( - indexer.net_value(&tx, 3..5).to_sat(), - (10_000 * (5 - 3)) as i64 + indexer.net_value(&tx, 3..6).to_sat(), + (10_000 * (6 - 3 - /*the skipped one*/ 1)) as i64 ); } diff --git a/crates/wallet/src/descriptor/error.rs b/crates/wallet/src/descriptor/error.rs index 48d0091d..f83fb24d 100644 --- a/crates/wallet/src/descriptor/error.rs +++ b/crates/wallet/src/descriptor/error.rs @@ -23,7 +23,6 @@ pub enum Error { HardenedDerivationXpub, /// The descriptor contains multipath keys MultiPath, - /// Error thrown while working with [`keys`](crate::keys) Key(crate::keys::KeyError), /// Error while extracting and manipulating policies diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 64d9cfa9..de7f19b0 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -772,7 +772,7 @@ impl Wallet { keychain: KeychainKind, index: u32, ) -> anyhow::Result + '_> { - let (spk_iter, index_changeset) = self + let (spks, index_changeset) = self .indexed_graph .index .reveal_to_target(&keychain, index) @@ -781,7 +781,7 @@ impl Wallet { self.persist .stage_and_commit(indexed_tx_graph::ChangeSet::from(index_changeset).into())?; - Ok(spk_iter.map(move |(index, spk)| AddressInfo { + Ok(spks.into_iter().map(move |(index, spk)| AddressInfo { index, address: Address::from_script(&spk, self.network).expect("must have address form"), keychain, @@ -861,7 +861,7 @@ impl Wallet { /// /// Will only return `Some(_)` if the wallet has given out the spk. pub fn derivation_of_spk(&self, spk: &Script) -> Option<(KeychainKind, u32)> { - self.indexed_graph.index.index_of_spk(spk) + self.indexed_graph.index.index_of_spk(spk).cloned() } /// Return the list of unspent outputs of this wallet @@ -871,7 +871,7 @@ impl Wallet { .filter_chain_unspents( &self.chain, self.chain.tip().block_id(), - self.indexed_graph.index.outpoints(), + self.indexed_graph.index.outpoints().iter().cloned(), ) .map(|((k, i), full_txo)| new_local_utxo(k, i, full_txo)) } @@ -885,7 +885,7 @@ impl Wallet { .filter_chain_txouts( &self.chain, self.chain.tip().block_id(), - self.indexed_graph.index.outpoints(), + self.indexed_graph.index.outpoints().iter().cloned(), ) .map(|((k, i), full_txo)| new_local_utxo(k, i, full_txo)) } @@ -932,7 +932,7 @@ impl Wallet { /// Returns the utxo owned by this wallet corresponding to `outpoint` if it exists in the /// wallet's database. pub fn get_utxo(&self, op: OutPoint) -> Option { - let (keychain, index, _) = self.indexed_graph.index.txout(op)?; + let (&(keychain, index), _) = self.indexed_graph.index.txout(op)?; self.indexed_graph .graph() .filter_chain_unspents( @@ -1207,7 +1207,7 @@ impl Wallet { self.indexed_graph.graph().balance( &self.chain, self.chain.tip().block_id(), - self.indexed_graph.index.outpoints(), + self.indexed_graph.index.outpoints().iter().cloned(), |&(k, _), _| k == KeychainKind::Internal, ) } @@ -1699,7 +1699,7 @@ impl Wallet { .into(); let weighted_utxo = match txout_index.index_of_spk(&txout.script_pubkey) { - Some((keychain, derivation_index)) => { + Some(&(keychain, derivation_index)) => { let satisfaction_weight = self .get_descriptor_for_keychain(keychain) .max_weight_to_satisfy() @@ -1744,7 +1744,7 @@ impl Wallet { for (index, txout) in tx.output.iter().enumerate() { let change_keychain = KeychainKind::Internal; match txout_index.index_of_spk(&txout.script_pubkey) { - Some((keychain, _)) if keychain == change_keychain => { + Some((keychain, _)) if *keychain == change_keychain => { change_index = Some(index) } _ => {} @@ -2015,13 +2015,13 @@ impl Wallet { if let Some((keychain, index)) = txout_index.index_of_spk(&txout.script_pubkey) { // NOTE: unmark_used will **not** make something unused if it has actually been used // by a tx in the tracker. It only removes the superficial marking. - txout_index.unmark_used(keychain, index); + txout_index.unmark_used(*keychain, *index); } } } fn get_descriptor_for_txout(&self, txout: &TxOut) -> Option { - let (keychain, child) = self + let &(keychain, child) = self .indexed_graph .index .index_of_spk(&txout.script_pubkey)?; @@ -2237,7 +2237,7 @@ impl Wallet { ) -> Result { // Try to find the prev_script in our db to figure out if this is internal or external, // and the derivation index - let (keychain, child) = self + let &(keychain, child) = self .indexed_graph .index .index_of_spk(&utxo.txout.script_pubkey) @@ -2285,7 +2285,7 @@ impl Wallet { // Try to figure out the keychain and derivation for every input and output for (is_input, index, out) in utxos.into_iter() { - if let Some((keychain, child)) = + if let Some(&(keychain, child)) = self.indexed_graph.index.index_of_spk(&out.script_pubkey) { let desc = self.get_descriptor_for_keychain(keychain); @@ -2331,7 +2331,7 @@ impl Wallet { None => ChangeSet::default(), }; - let (_, index_changeset) = self + let index_changeset = self .indexed_graph .index .reveal_to_target_multi(&update.last_active_indices); @@ -2536,17 +2536,27 @@ fn create_signers( ) -> Result<(Arc, Arc), DescriptorError> { let descriptor = into_wallet_descriptor_checked(descriptor, secp, network)?; let change_descriptor = into_wallet_descriptor_checked(change_descriptor, secp, network)?; - if descriptor.0 == change_descriptor.0 { - return Err(DescriptorError::ExternalAndInternalAreTheSame); - } - let (descriptor, keymap) = descriptor; let signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp)); - let _ = index.insert_descriptor(KeychainKind::External, descriptor); + let _ = index + .insert_descriptor(KeychainKind::External, descriptor) + .expect("this is the first descriptor we're inserting"); let (descriptor, keymap) = change_descriptor; let change_signers = Arc::new(SignersContainer::build(keymap, &descriptor, secp)); - let _ = index.insert_descriptor(KeychainKind::Internal, descriptor); + let _ = index + .insert_descriptor(KeychainKind::Internal, descriptor) + .map_err(|e| { + use bdk_chain::keychain::InsertDescriptorError; + match e { + InsertDescriptorError::DescriptorAlreadyAssigned { .. } => { + crate::descriptor::error::Error::ExternalAndInternalAreTheSame + } + InsertDescriptorError::KeychainAlreadyAssigned { .. } => { + unreachable!("this is the first time we're assigning internal") + } + } + })?; Ok((signers, change_signers)) } diff --git a/example-crates/example_bitcoind_rpc_polling/src/main.rs b/example-crates/example_bitcoind_rpc_polling/src/main.rs index a921962c..be9e1839 100644 --- a/example-crates/example_bitcoind_rpc_polling/src/main.rs +++ b/example-crates/example_bitcoind_rpc_polling/src/main.rs @@ -212,7 +212,7 @@ fn main() -> anyhow::Result<()> { graph.graph().balance( &*chain, synced_to.block_id(), - graph.index.outpoints(), + graph.index.outpoints().iter().cloned(), |(k, _), _| k == &Keychain::Internal, ) }; @@ -336,7 +336,7 @@ fn main() -> anyhow::Result<()> { graph.graph().balance( &*chain, synced_to.block_id(), - graph.index.outpoints(), + graph.index.outpoints().iter().cloned(), |(k, _), _| k == &Keychain::Internal, ) }; diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 256c4bdf..5355f556 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -427,7 +427,7 @@ pub fn planned_utxos Option, _>> { let (k, i, full_txo) = match r { Err(err) => return Some(Err(err)), @@ -527,7 +527,7 @@ where let balance = graph.graph().try_balance( chain, chain.get_chain_tip()?, - graph.index.outpoints(), + graph.index.outpoints().iter().cloned(), |(k, _), _| k == &Keychain::Internal, )?; @@ -568,7 +568,7 @@ where } => { let txouts = graph .graph() - .try_filter_chain_txouts(chain, chain_tip, outpoints) + .try_filter_chain_txouts(chain, chain_tip, outpoints.iter().cloned()) .filter(|r| match r { Ok((_, full_txo)) => match (spent, unspent) { (true, false) => full_txo.spent_by.is_some(), diff --git a/example-crates/example_electrum/src/main.rs b/example-crates/example_electrum/src/main.rs index 8467d269..175b7ce5 100644 --- a/example-crates/example_electrum/src/main.rs +++ b/example-crates/example_electrum/src/main.rs @@ -228,9 +228,9 @@ fn main() -> anyhow::Result<()> { let all_spks = graph .index .revealed_spks(..) - .map(|(k, i, spk)| (k.to_owned(), i, spk.to_owned())) + .map(|(index, spk)| (index.to_owned(), spk.to_owned())) .collect::>(); - request = request.chain_spks(all_spks.into_iter().map(|(k, spk_i, spk)| { + request = request.chain_spks(all_spks.into_iter().map(|((k, spk_i), spk)| { eprint!("Scanning {}: {}", k, spk_i); spk })); @@ -239,10 +239,10 @@ fn main() -> anyhow::Result<()> { let unused_spks = graph .index .unused_spks() - .map(|(k, i, spk)| (k, i, spk.to_owned())) + .map(|(index, spk)| (index.to_owned(), spk.to_owned())) .collect::>(); request = - request.chain_spks(unused_spks.into_iter().map(move |(k, spk_i, spk)| { + request.chain_spks(unused_spks.into_iter().map(move |((k, spk_i), spk)| { eprint!( "Checking if address {} {}:{} has been used", Address::from_script(&spk, args.network).unwrap(), @@ -258,7 +258,11 @@ fn main() -> anyhow::Result<()> { let utxos = graph .graph() - .filter_chain_unspents(&*chain, chain_tip.block_id(), init_outpoints) + .filter_chain_unspents( + &*chain, + chain_tip.block_id(), + init_outpoints.iter().cloned(), + ) .map(|(_, utxo)| utxo) .collect::>(); request = request.chain_outpoints(utxos.into_iter().map(|utxo| { @@ -338,7 +342,7 @@ fn main() -> anyhow::Result<()> { let mut indexed_tx_graph_changeset = indexed_tx_graph::ChangeSet::::default(); if let Some(keychain_update) = keychain_update { - let (_, keychain_changeset) = graph.index.reveal_to_target_multi(&keychain_update); + let keychain_changeset = graph.index.reveal_to_target_multi(&keychain_update); indexed_tx_graph_changeset.append(keychain_changeset.into()); } indexed_tx_graph_changeset.append(graph.apply_update(graph_update)); diff --git a/example-crates/example_esplora/src/main.rs b/example-crates/example_esplora/src/main.rs index 3273787f..d0b16726 100644 --- a/example-crates/example_esplora/src/main.rs +++ b/example-crates/example_esplora/src/main.rs @@ -204,7 +204,7 @@ fn main() -> anyhow::Result<()> { // addresses derived so we need to derive up to last active addresses the scan found // before adding the transactions. (chain.apply_update(update.chain_update)?, { - let (_, index_changeset) = graph + let index_changeset = graph .index .reveal_to_target_multi(&update.last_active_indices); let mut indexed_tx_graph_changeset = graph.apply_update(update.graph_update); @@ -245,7 +245,7 @@ fn main() -> anyhow::Result<()> { let all_spks = graph .index .revealed_spks(..) - .map(|(k, i, spk)| (k.to_owned(), i, spk.to_owned())) + .map(|((k, i), spk)| (k.to_owned(), *i, spk.to_owned())) .collect::>(); request = request.chain_spks(all_spks.into_iter().map(|(k, i, spk)| { eprint!("scanning {}:{}", k, i); @@ -258,10 +258,10 @@ fn main() -> anyhow::Result<()> { let unused_spks = graph .index .unused_spks() - .map(|(k, i, spk)| (k, i, spk.to_owned())) + .map(|(index, spk)| (index.to_owned(), spk.to_owned())) .collect::>(); request = - request.chain_spks(unused_spks.into_iter().map(move |(k, i, spk)| { + request.chain_spks(unused_spks.into_iter().map(move |((k, i), spk)| { eprint!( "Checking if address {} {}:{} has been used", Address::from_script(&spk, args.network).unwrap(), @@ -280,7 +280,11 @@ fn main() -> anyhow::Result<()> { let init_outpoints = graph.index.outpoints(); let utxos = graph .graph() - .filter_chain_unspents(&*chain, local_tip.block_id(), init_outpoints) + .filter_chain_unspents( + &*chain, + local_tip.block_id(), + init_outpoints.iter().cloned(), + ) .map(|(_, utxo)| utxo) .collect::>(); request = request.chain_outpoints( From 4d2442c37f5c1bd822795271a79676d1ffbe7916 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 6 Jun 2024 13:12:38 +1000 Subject: [PATCH 03/14] chore(chain): misc docs and insert_descriptor fixes --- crates/chain/src/keychain/txout_index.rs | 47 +++++-------------- crates/chain/src/spk_iter.rs | 8 +++- crates/chain/tests/test_indexed_tx_graph.rs | 5 +- .../chain/tests/test_keychain_txout_index.rs | 18 +++++-- example-crates/example_cli/src/lib.rs | 4 +- 5 files changed, 36 insertions(+), 46 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index f4f736a4..846b6c24 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -95,7 +95,8 @@ impl Default for ChangeSet { } } -const DEFAULT_LOOKAHEAD: u32 = 25; +/// The default lookahead for a [`KeychainTxOutIndex`] +pub const DEFAULT_LOOKAHEAD: u32 = 25; /// [`KeychainTxOutIndex`] controls how script pubkeys are revealed for multiple keychains, and /// indexes [`TxOut`]s with them. @@ -121,15 +122,17 @@ const DEFAULT_LOOKAHEAD: u32 = 25; /// above the last revealed index. These additionally-derived script pubkeys are called the /// lookahead. /// -/// The [`KeychainTxOutIndex`] is constructed with the `lookahead` and cannot be altered. The -/// default `lookahead` count is 1000. Use [`new`] to set a custom `lookahead`. +/// The [`KeychainTxOutIndex`] is constructed with the `lookahead` and cannot be altered. See +/// [`DEFAULT_LOOKAHEAD`] for the value used in the `Default` implementation. Use [`new`] to set a +/// custom `lookahead`. /// /// # Unbounded script pubkey iterator /// /// For script-pubkey-based chain sources (such as Electrum/Esplora), an initial scan is best done /// by iterating though derived script pubkeys one by one and requesting transaction histories for /// each script pubkey. We will stop after x-number of script pubkeys have empty histories. An -/// unbounded script pubkey iterator is useful to pass to such a chain source. +/// unbounded script pubkey iterator is useful to pass to such a chain source because it doesn't +/// require holding a reference to the index. /// /// Call [`unbounded_spk_iter`] to get an unbounded script pubkey iterator for a given keychain. /// Call [`all_unbounded_spk_iters`] to get unbounded script pubkey iterators for all keychains. @@ -162,42 +165,14 @@ const DEFAULT_LOOKAHEAD: u32 = 25; /// # let (external_descriptor,_) = Descriptor::::parse_descriptor(&secp, "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/0/*)").unwrap(); /// # let (internal_descriptor,_) = Descriptor::::parse_descriptor(&secp, "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/1/*)").unwrap(); /// # let (descriptor_42, _) = Descriptor::::parse_descriptor(&secp, "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/2/*)").unwrap(); -/// let _ = txout_index.insert_descriptor(MyKeychain::External, external_descriptor); -/// let _ = txout_index.insert_descriptor(MyKeychain::Internal, internal_descriptor); -/// let _ = txout_index.insert_descriptor(MyKeychain::MyAppUser { user_id: 42 }, descriptor_42); +/// let _ = txout_index.insert_descriptor(MyKeychain::External, external_descriptor)?; +/// let _ = txout_index.insert_descriptor(MyKeychain::Internal, internal_descriptor)?; +/// let _ = txout_index.insert_descriptor(MyKeychain::MyAppUser { user_id: 42 }, descriptor_42)?; /// /// let new_spk_for_user = txout_index.reveal_next_spk(&MyKeychain::MyAppUser{ user_id: 42 }); +/// # Ok::<_, bdk_chain::keychain::InsertDescriptorError<_>>(()) /// ``` /// -/// # Non-recommend keychain to descriptor assignments -/// -/// A keychain (`K`) is used to identify a descriptor. However, the following keychain to descriptor -/// arrangements result in behavior that is harder to reason about and is not recommended. -/// -/// ## Multiple keychains identifying the same descriptor -/// -/// Although a single keychain variant can only identify a single descriptor, multiple keychain -/// variants can identify the same descriptor. -/// -/// If multiple keychains identify the same descriptor: -/// 1. Methods that take in a keychain (such as [`reveal_next_spk`]) will work normally when any -/// keychain (that identifies that descriptor) is passed in. -/// 2. Methods that return data which associates with a descriptor (such as [`outpoints`], -/// [`txouts`], [`unused_spks`], etc.) the method will return the highest-ranked keychain variant -/// that identifies the descriptor. Rank is determined by the [`Ord`] implementation of the keychain -/// type. -/// -/// This arrangement is not recommended since some methods will return a single keychain variant -/// even though multiple keychain variants identify the same descriptor. -/// -/// ## Reassigning the descriptor of a single keychain -/// -/// Descriptors added to [`KeychainTxOutIndex`] are never removed. However, a keychain that -/// identifies a descriptor can be reassigned to identify a different descriptor. This may result in -/// a situation where a descriptor has no associated keychain(s), and relevant [`TxOut`]s, -/// [`OutPoint`]s and [`Script`]s (of that descriptor) will not be return by [`KeychainTxOutIndex`]. -/// Therefore, reassigning the descriptor of a single keychain is not recommended. -/// /// [`Ord`]: core::cmp::Ord /// [`SpkTxOutIndex`]: crate::spk_txout_index::SpkTxOutIndex /// [`Descriptor`]: crate::miniscript::Descriptor diff --git a/crates/chain/src/spk_iter.rs b/crates/chain/src/spk_iter.rs index d3a37279..91c271c7 100644 --- a/crates/chain/src/spk_iter.rs +++ b/crates/chain/src/spk_iter.rs @@ -158,8 +158,12 @@ mod test { let (external_descriptor,_) = Descriptor::::parse_descriptor(&secp, "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/0/*)").unwrap(); let (internal_descriptor,_) = Descriptor::::parse_descriptor(&secp, "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/1/*)").unwrap(); - let _ = txout_index.insert_descriptor(TestKeychain::External, external_descriptor.clone()); - let _ = txout_index.insert_descriptor(TestKeychain::Internal, internal_descriptor.clone()); + let _ = txout_index + .insert_descriptor(TestKeychain::External, external_descriptor.clone()) + .unwrap(); + let _ = txout_index + .insert_descriptor(TestKeychain::Internal, internal_descriptor.clone()) + .unwrap(); (txout_index, external_descriptor, internal_descriptor) } diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index f386f0cf..4266e184 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -34,7 +34,10 @@ fn insert_relevant_txs() { let mut graph = IndexedTxGraph::>::new( KeychainTxOutIndex::new(10), ); - let _ = graph.index.insert_descriptor((), descriptor.clone()); + let _ = graph + .index + .insert_descriptor((), descriptor.clone()) + .unwrap(); let tx_a = Transaction { output: vec![ diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index b2bdba5a..c5fd974c 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -34,8 +34,12 @@ fn init_txout_index( ) -> bdk_chain::keychain::KeychainTxOutIndex { let mut txout_index = bdk_chain::keychain::KeychainTxOutIndex::::new(lookahead); - let _ = txout_index.insert_descriptor(TestKeychain::External, external_descriptor); - let _ = txout_index.insert_descriptor(TestKeychain::Internal, internal_descriptor); + let _ = txout_index + .insert_descriptor(TestKeychain::External, external_descriptor) + .unwrap(); + let _ = txout_index + .insert_descriptor(TestKeychain::Internal, internal_descriptor) + .unwrap(); txout_index } @@ -458,7 +462,9 @@ fn test_non_wildcard_derivations() { .unwrap() .script_pubkey(); - let _ = txout_index.insert_descriptor(TestKeychain::External, no_wildcard_descriptor.clone()); + let _ = txout_index + .insert_descriptor(TestKeychain::External, no_wildcard_descriptor.clone()) + .unwrap(); // given: // - `txout_index` with no stored scripts @@ -702,7 +708,9 @@ fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() { fn assigning_same_descriptor_to_multiple_keychains_should_error() { let desc = parse_descriptor(DESCRIPTORS[0]); let mut indexer = KeychainTxOutIndex::::new(0); - let _ = indexer.insert_descriptor(TestKeychain::Internal, desc.clone()); + let _ = indexer + .insert_descriptor(TestKeychain::Internal, desc.clone()) + .unwrap(); assert!(indexer .insert_descriptor(TestKeychain::External, desc) .is_err()) @@ -726,7 +734,7 @@ fn when_querying_over_a_range_of_keychains_the_utxos_should_show_up() { for (i, descriptor) in DESCRIPTORS.iter().enumerate() { let descriptor = parse_descriptor(descriptor); - let _ = indexer.insert_descriptor(i, descriptor.clone()); + let _ = indexer.insert_descriptor(i, descriptor.clone()).unwrap(); if i != 4 { // skip one in the middle to see if uncovers any bugs indexer.reveal_next_spk(&i); diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 5355f556..993bde41 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -709,7 +709,7 @@ where // them in the index here. However, the keymap is not stored in the database. let (descriptor, mut keymap) = Descriptor::::parse_descriptor(&secp, &args.descriptor)?; - let _ = index.insert_descriptor(Keychain::External, descriptor); + let _ = index.insert_descriptor(Keychain::External, descriptor)?; if let Some((internal_descriptor, internal_keymap)) = args .change_descriptor @@ -718,7 +718,7 @@ where .transpose()? { keymap.extend(internal_keymap); - let _ = index.insert_descriptor(Keychain::Internal, internal_descriptor); + let _ = index.insert_descriptor(Keychain::Internal, internal_descriptor)?; } let mut db_backend = match Store::::open_or_create_new(db_magic, &args.db_path) { From bce070b1d662db7ac120e1d236fdda51842ad738 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Thu, 6 Jun 2024 14:10:20 -0500 Subject: [PATCH 04/14] chore(chain): add type IndexSpk, fix clippy type complexity warning --- clippy.toml | 3 +-- crates/chain/src/keychain/txout_index.rs | 8 ++++---- crates/chain/src/spk_client.rs | 8 ++++---- crates/chain/src/spk_iter.rs | 5 ++++- crates/esplora/src/async_ext.rs | 4 ++-- crates/esplora/src/blocking_ext.rs | 4 ++-- crates/wallet/src/wallet/mod.rs | 6 +++--- 7 files changed, 20 insertions(+), 18 deletions(-) diff --git a/clippy.toml b/clippy.toml index fec36d9d..e3b99604 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,2 +1 @@ -msrv="1.63.0" -type-complexity-threshold = 275 +msrv="1.63.0" \ No newline at end of file diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 846b6c24..6d0b7623 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -3,10 +3,10 @@ use crate::{ indexed_tx_graph::Indexer, miniscript::{Descriptor, DescriptorPublicKey}, spk_iter::BIP32_MAX_INDEX, - DescriptorExt, DescriptorId, SpkIterator, SpkTxOutIndex, + DescriptorExt, DescriptorId, IndexSpk, SpkIterator, SpkTxOutIndex, }; use alloc::{borrow::ToOwned, vec::Vec}; -use bitcoin::{Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; +use bitcoin::{Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid}; use core::{ fmt::Debug, ops::{Bound, RangeBounds}, @@ -739,9 +739,9 @@ impl KeychainTxOutIndex { &mut self, keychain: &K, target_index: u32, - ) -> Option<(Vec<(u32, ScriptBuf)>, ChangeSet)> { + ) -> Option<(Vec, ChangeSet)> { let mut changeset = ChangeSet::default(); - let mut spks: Vec<(u32, ScriptBuf)> = vec![]; + let mut spks: Vec = vec![]; while let Some((i, new)) = self.next_index(keychain) { if !new || i > target_index { break; diff --git a/crates/chain/src/spk_client.rs b/crates/chain/src/spk_client.rs index d65ecdba..cf78e7bd 100644 --- a/crates/chain/src/spk_client.rs +++ b/crates/chain/src/spk_client.rs @@ -1,7 +1,7 @@ //! Helper types for spk-based blockchain clients. use crate::{ - collections::BTreeMap, local_chain::CheckPoint, ConfirmationTimeHeightAnchor, TxGraph, + collections::BTreeMap, local_chain::CheckPoint, ConfirmationTimeHeightAnchor, IndexSpk, TxGraph, }; use alloc::{boxed::Box, vec::Vec}; use bitcoin::{OutPoint, Script, ScriptBuf, Txid}; @@ -195,7 +195,7 @@ pub struct FullScanRequest { /// [`LocalChain::tip`]: crate::local_chain::LocalChain::tip pub chain_tip: CheckPoint, /// Iterators of script pubkeys indexed by the keychain index. - pub spks_by_keychain: BTreeMap + Send>>, + pub spks_by_keychain: BTreeMap + Send>>, } impl FullScanRequest { @@ -238,7 +238,7 @@ impl FullScanRequest { pub fn set_spks_for_keychain( mut self, keychain: K, - spks: impl IntoIterator + Send + 'static>, + spks: impl IntoIterator + Send + 'static>, ) -> Self { self.spks_by_keychain .insert(keychain, Box::new(spks.into_iter())); @@ -252,7 +252,7 @@ impl FullScanRequest { pub fn chain_spks_for_keychain( mut self, keychain: K, - spks: impl IntoIterator + Send + 'static>, + spks: impl IntoIterator + Send + 'static>, ) -> Self { match self.spks_by_keychain.remove(&keychain) { // clippy here suggests to remove `into_iter` from `spks.into_iter()`, but doing so diff --git a/crates/chain/src/spk_iter.rs b/crates/chain/src/spk_iter.rs index 91c271c7..8285036b 100644 --- a/crates/chain/src/spk_iter.rs +++ b/crates/chain/src/spk_iter.rs @@ -7,6 +7,9 @@ use core::{borrow::Borrow, ops::Bound, ops::RangeBounds}; /// Maximum [BIP32](https://bips.xyz/32) derivation index. pub const BIP32_MAX_INDEX: u32 = (1 << 31) - 1; +/// A tuple of keychain index and corresponding [`ScriptBuf`]. +pub type IndexSpk = (u32, ScriptBuf); + /// An iterator for derived script pubkeys. /// /// [`SpkIterator`] is an implementation of the [`Iterator`] trait which possesses its own `next()` @@ -97,7 +100,7 @@ impl Iterator for SpkIterator where D: Borrow>, { - type Item = (u32, ScriptBuf); + type Item = IndexSpk; fn next(&mut self) -> Option { // For non-wildcard descriptors, we expect the first element to be Some((0, spk)), then None after. diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index 304dedb3..a7dbc24f 100644 --- a/crates/esplora/src/async_ext.rs +++ b/crates/esplora/src/async_ext.rs @@ -2,13 +2,13 @@ use std::collections::BTreeSet; use async_trait::async_trait; use bdk_chain::spk_client::{FullScanRequest, FullScanResult, SyncRequest, SyncResult}; -use bdk_chain::Anchor; use bdk_chain::{ bitcoin::{BlockHash, OutPoint, ScriptBuf, TxOut, Txid}, collections::BTreeMap, local_chain::CheckPoint, BlockId, ConfirmationTimeHeightAnchor, TxGraph, }; +use bdk_chain::{Anchor, IndexSpk}; use esplora_client::{Amount, TxStatus}; use futures::{stream::FuturesOrdered, TryStreamExt}; @@ -236,7 +236,7 @@ async fn full_scan_for_index_and_graph( client: &esplora_client::AsyncClient, keychain_spks: BTreeMap< K, - impl IntoIterator + Send> + Send, + impl IntoIterator + Send> + Send, >, stop_gap: usize, parallel_requests: usize, diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 09ad9c0a..0b173c3a 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/crates/esplora/src/blocking_ext.rs @@ -4,12 +4,12 @@ use std::usize; use bdk_chain::collections::BTreeMap; use bdk_chain::spk_client::{FullScanRequest, FullScanResult, SyncRequest, SyncResult}; -use bdk_chain::Anchor; use bdk_chain::{ bitcoin::{Amount, BlockHash, OutPoint, ScriptBuf, TxOut, Txid}, local_chain::CheckPoint, BlockId, ConfirmationTimeHeightAnchor, TxGraph, }; +use bdk_chain::{Anchor, IndexSpk}; use esplora_client::TxStatus; use crate::anchor_from_status; @@ -217,7 +217,7 @@ fn chain_update( /// [`KeychainTxOutIndex`](bdk_chain::keychain::KeychainTxOutIndex). fn full_scan_for_index_and_graph_blocking( client: &esplora_client::BlockingClient, - keychain_spks: BTreeMap>, + keychain_spks: BTreeMap>, stop_gap: usize, parallel_requests: usize, ) -> Result<(TxGraph, BTreeMap), Error> { diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index de7f19b0..18498412 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -29,7 +29,7 @@ use bdk_chain::{ spk_client::{FullScanRequest, FullScanResult, SyncRequest, SyncResult}, tx_graph::{CanonicalTx, TxGraph}, Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeHeightAnchor, FullTxOut, - IndexedTxGraph, + IndexSpk, IndexedTxGraph, }; use bdk_persist::{Persist, PersistBackend}; use bitcoin::secp256k1::{All, Secp256k1}; @@ -910,7 +910,7 @@ impl Wallet { /// script pubkeys the wallet is storing internally). pub fn all_unbounded_spk_iters( &self, - ) -> BTreeMap + Clone> { + ) -> BTreeMap + Clone> { self.indexed_graph.index.all_unbounded_spk_iters() } @@ -922,7 +922,7 @@ impl Wallet { pub fn unbounded_spk_iter( &self, keychain: KeychainKind, - ) -> impl Iterator + Clone { + ) -> impl Iterator + Clone { self.indexed_graph .index .unbounded_spk_iter(&keychain) From 101a09a97fa5e8d675c13396b9a800665b1b6c22 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Fri, 7 Jun 2024 12:29:58 +1000 Subject: [PATCH 05/14] chore(chain): Standardise KeychainTxOutIndex return types The previous commit b9c5b9d08b040faf6c6b2d9b3745918031555b72 added IndexSpk. This goes further and adds `Indexed` and `KeychainIndexed` type alises (IndexSpk is Indexed) and attempts to standardize the structure of return types more generally. --- crates/chain/src/keychain.rs | 7 +- crates/chain/src/keychain/txout_index.rs | 114 ++++++++++-------- crates/chain/src/lib.rs | 1 + crates/chain/src/spk_client.rs | 9 +- crates/chain/src/spk_iter.rs | 6 +- .../chain/tests/test_keychain_txout_index.rs | 18 +-- crates/esplora/src/async_ext.rs | 4 +- crates/esplora/src/blocking_ext.rs | 4 +- crates/wallet/src/wallet/mod.rs | 15 +-- example-crates/example_cli/src/lib.rs | 7 +- example-crates/example_electrum/src/main.rs | 4 +- example-crates/example_esplora/src/main.rs | 4 +- 12 files changed, 103 insertions(+), 90 deletions(-) diff --git a/crates/chain/src/keychain.rs b/crates/chain/src/keychain.rs index b822dc90..e1e6de68 100644 --- a/crates/chain/src/keychain.rs +++ b/crates/chain/src/keychain.rs @@ -12,7 +12,7 @@ #[cfg(feature = "miniscript")] mod txout_index; -use bitcoin::Amount; +use bitcoin::{Amount, ScriptBuf}; #[cfg(feature = "miniscript")] pub use txout_index::*; @@ -49,6 +49,11 @@ impl Balance { } } +/// A tuple of keychain index and corresponding [`ScriptBuf`]. +pub type Indexed = (u32, T); +/// A tuple of keychain, index and the [`ScriptBuf`] derived at that location. +pub type KeychainIndexed = ((K, u32), T); + impl core::fmt::Display for Balance { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 6d0b7623..f62c2f16 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -3,7 +3,7 @@ use crate::{ indexed_tx_graph::Indexer, miniscript::{Descriptor, DescriptorPublicKey}, spk_iter::BIP32_MAX_INDEX, - DescriptorExt, DescriptorId, IndexSpk, SpkIterator, SpkTxOutIndex, + DescriptorExt, DescriptorId, SpkIterator, SpkTxOutIndex, }; use alloc::{borrow::ToOwned, vec::Vec}; use bitcoin::{Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid}; @@ -12,6 +12,7 @@ use core::{ ops::{Bound, RangeBounds}, }; +use super::*; use crate::Append; /// Represents updates to the derivation index of a [`KeychainTxOutIndex`]. @@ -140,7 +141,7 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; /// # Change sets /// /// Methods that can update the last revealed index or add keychains will return [`ChangeSet`] to report -/// these changes. This can be persisted for future recovery. +/// these changes. This should be persisted for future recovery. /// /// ## Synopsis /// @@ -191,18 +192,18 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; #[derive(Clone, Debug)] pub struct KeychainTxOutIndex { inner: SpkTxOutIndex<(K, u32)>, - // keychain -> (descriptor, descriptor id) map + /// keychain -> (descriptor id) map keychains_to_descriptor_ids: BTreeMap, - // descriptor id -> keychain map + /// descriptor id -> keychain map descriptor_ids_to_keychains: BTreeMap, - // descriptor_id -> descriptor map - // This is a "monotone" map, meaning that its size keeps growing, i.e., we never delete - // descriptors from it. This is useful for revealing spks for descriptors that don't have - // keychains associated. + /// descriptor_id -> descriptor map + /// This is a "monotone" map, meaning that its size keeps growing, i.e., we never delete + /// descriptors from it. This is useful for revealing spks for descriptors that don't have + /// keychains associated. descriptor_ids_to_descriptors: BTreeMap>, - // last revealed indexes + /// last revealed indices for each descriptor. last_revealed: HashMap, - // lookahead settings for each keychain + /// lookahead setting lookahead: u32, } @@ -292,21 +293,28 @@ impl KeychainTxOutIndex { } /// Get the set of indexed outpoints, corresponding to tracked keychains. - pub fn outpoints(&self) -> &BTreeSet<((K, u32), OutPoint)> { + pub fn outpoints(&self) -> &BTreeSet> { self.inner.outpoints() } /// Iterate over known txouts that spend to tracked script pubkeys. - pub fn txouts(&self) -> impl DoubleEndedIterator + '_ { - self.inner.txouts() + pub fn txouts( + &self, + ) -> impl DoubleEndedIterator> + ExactSizeIterator + { + self.inner + .txouts() + .map(|(index, op, txout)| (index.clone(), (op, txout))) } /// Finds all txouts on a transaction that has previously been scanned and indexed. pub fn txouts_in_tx( &self, txid: Txid, - ) -> impl DoubleEndedIterator { - self.inner.txouts_in_tx(txid) + ) -> impl DoubleEndedIterator> { + self.inner + .txouts_in_tx(txid) + .map(|(index, op, txout)| (index.clone(), (op, txout))) } /// Return the [`TxOut`] of `outpoint` if it has been indexed, and if it corresponds to a @@ -315,8 +323,10 @@ impl KeychainTxOutIndex { /// The associated keychain and keychain index of the txout's spk is also returned. /// /// This calls [`SpkTxOutIndex::txout`] internally. - pub fn txout(&self, outpoint: OutPoint) -> Option<(&(K, u32), &TxOut)> { - self.inner.txout(outpoint) + pub fn txout(&self, outpoint: OutPoint) -> Option> { + self.inner + .txout(outpoint) + .map(|(index, txout)| (index.clone(), txout)) } /// Return the script that exists under the given `keychain`'s `index`. @@ -420,12 +430,12 @@ impl KeychainTxOutIndex { /// Insert a descriptor with a keychain associated to it. /// - /// Adding a descriptor means you will be able to derive new script pubkeys under it - /// and the txout index will discover transaction outputs with those script pubkeys. + /// Adding a descriptor means you will be able to derive new script pubkeys under it and the + /// txout index will discover transaction outputs with those script pubkeys (once they've been + /// derived and added to the index). /// - /// keychain <-> descriptor is a one-to-one mapping -- in `--release` this method will ignore calls that try to - /// associate a keychain with the descriptor of another keychain or to re-assign the keychain to - /// new descriptor. If `debug_assertions` are enabled then it will panic. + /// keychain <-> descriptor is a one-to-one mapping that cannot be changed. Attempting to do so + /// will return a [`InsertDescriptorError`]. pub fn insert_descriptor( &mut self, keychain: K, @@ -575,7 +585,7 @@ impl KeychainTxOutIndex { pub fn revealed_spks( &self, range: impl RangeBounds, - ) -> impl Iterator { + ) -> impl Iterator> { let start = range.start_bound(); let end = range.end_bound(); let mut iter_last_revealed = self @@ -593,8 +603,7 @@ impl KeychainTxOutIndex { // iterate through the last_revealed for each keychain and the spks for each keychain in // tandem. This minimizes BTreeMap queries. core::iter::from_fn(move || loop { - let (path, spk) = iter_spks.next()?; - let (keychain, index) = path; + let ((keychain, index), spk) = iter_spks.next()?; // We need to find the last revealed that matches the current spk we are considering so // we skip ahead. while current_keychain?.0 < keychain { @@ -603,7 +612,7 @@ impl KeychainTxOutIndex { let (current_keychain, last_revealed) = current_keychain?; if current_keychain == keychain && Some(*index) <= last_revealed { - break Some((path, spk.as_script())); + break Some(((keychain.clone(), *index), spk.as_script())); } }) } @@ -615,7 +624,7 @@ impl KeychainTxOutIndex { pub fn revealed_keychain_spks<'a>( &'a self, keychain: &'a K, - ) -> impl DoubleEndedIterator + 'a { + ) -> impl DoubleEndedIterator> + 'a { let end = self .last_revealed_index(keychain) .map(|v| v + 1) @@ -627,7 +636,9 @@ impl KeychainTxOutIndex { } /// Iterate over revealed, but unused, spks of all keychains. - pub fn unused_spks(&self) -> impl DoubleEndedIterator + Clone { + pub fn unused_spks( + &self, + ) -> impl DoubleEndedIterator> + Clone { self.keychains_to_descriptor_ids .keys() .flat_map(|keychain| { @@ -641,7 +652,7 @@ impl KeychainTxOutIndex { pub fn unused_keychain_spks( &self, keychain: &K, - ) -> impl DoubleEndedIterator + Clone { + ) -> impl DoubleEndedIterator> + Clone { let end = match self.keychains_to_descriptor_ids.get(keychain) { Some(did) => self.last_revealed.get(did).map(|v| *v + 1).unwrap_or(0), None => 0, @@ -739,16 +750,16 @@ impl KeychainTxOutIndex { &mut self, keychain: &K, target_index: u32, - ) -> Option<(Vec, ChangeSet)> { + ) -> Option<(Vec>, ChangeSet)> { let mut changeset = ChangeSet::default(); - let mut spks: Vec = vec![]; + let mut spks: Vec> = vec![]; while let Some((i, new)) = self.next_index(keychain) { if !new || i > target_index { break; } match self.reveal_next_spk(keychain) { Some(((i, spk), change)) => { - spks.push((i, spk.to_owned())); + spks.push((i, spk)); changeset.append(change); } None => break, @@ -770,7 +781,7 @@ impl KeychainTxOutIndex { /// 1. The descriptor has no wildcard and already has one script revealed. /// 2. The descriptor has already revealed scripts up to the numeric bound. /// 3. There is no descriptor associated with the given keychain. - pub fn reveal_next_spk(&mut self, keychain: &K) -> Option<((u32, &Script), ChangeSet)> { + pub fn reveal_next_spk(&mut self, keychain: &K) -> Option<(Indexed, ChangeSet)> { let (next_index, new) = self.next_index(keychain)?; let mut changeset = ChangeSet::default(); @@ -790,7 +801,7 @@ impl KeychainTxOutIndex { .inner .spk_at_index(&(keychain.clone(), next_index)) .expect("we just inserted it"); - Some(((next_index, script), changeset)) + Some(((next_index, script.into()), changeset)) } /// Gets the next unused script pubkey in the keychain. I.e., the script pubkey with the lowest @@ -802,20 +813,17 @@ impl KeychainTxOutIndex { /// has used all scripts up to the derivation bounds, then the last derived script pubkey will be /// returned. /// - /// Returns None if the provided keychain doesn't exist. - pub fn next_unused_spk(&mut self, keychain: &K) -> Option<((u32, &Script), ChangeSet)> { - let need_new = self.unused_keychain_spks(keychain).next().is_none(); - // this rather strange branch is needed because of some lifetime issues - if need_new { - self.reveal_next_spk(keychain) - } else { - Some(( - self.unused_keychain_spks(keychain) - .next() - .expect("we already know next exists"), - ChangeSet::default(), - )) - } + /// Returns `None` if there are no script pubkeys that have been used and no new script pubkey + /// could be revealed (see [`reveal_next_spk`] for when this happens). + /// + /// [`reveal_next_spk`]: Self::reveal_next_spk + pub fn next_unused_spk(&mut self, keychain: &K) -> Option<(Indexed, ChangeSet)> { + let next_unused = self + .unused_keychain_spks(keychain) + .next() + .map(|(i, spk)| ((i, spk.to_owned()), ChangeSet::default())); + + next_unused.or_else(|| self.reveal_next_spk(keychain)) } /// Iterate over all [`OutPoint`]s that have `TxOut`s with script pubkeys derived from @@ -823,17 +831,19 @@ impl KeychainTxOutIndex { pub fn keychain_outpoints<'a>( &'a self, keychain: &'a K, - ) -> impl DoubleEndedIterator + 'a { + ) -> impl DoubleEndedIterator> + 'a { self.keychain_outpoints_in_range(keychain..=keychain) - .map(|((_, i), op)| (*i, op)) + .map(|((_, i), op)| (i, op)) } /// Iterate over [`OutPoint`]s that have script pubkeys derived from keychains in `range`. pub fn keychain_outpoints_in_range<'a>( &'a self, range: impl RangeBounds + 'a, - ) -> impl DoubleEndedIterator + 'a { - self.inner.outputs_in_range(self.map_to_inner_bounds(range)) + ) -> impl DoubleEndedIterator> + 'a { + self.inner + .outputs_in_range(self.map_to_inner_bounds(range)) + .map(|((k, i), op)| ((k.clone(), *i), op)) } fn map_to_inner_bounds(&self, bound: impl RangeBounds) -> impl RangeBounds<(K, u32)> { diff --git a/crates/chain/src/lib.rs b/crates/chain/src/lib.rs index 97096994..8204cf0e 100644 --- a/crates/chain/src/lib.rs +++ b/crates/chain/src/lib.rs @@ -28,6 +28,7 @@ pub use chain_data::*; pub mod indexed_tx_graph; pub use indexed_tx_graph::IndexedTxGraph; pub mod keychain; +pub use keychain::{Indexed, KeychainIndexed}; pub mod local_chain; mod tx_data_traits; pub mod tx_graph; diff --git a/crates/chain/src/spk_client.rs b/crates/chain/src/spk_client.rs index cf78e7bd..3cb87a40 100644 --- a/crates/chain/src/spk_client.rs +++ b/crates/chain/src/spk_client.rs @@ -1,7 +1,8 @@ //! Helper types for spk-based blockchain clients. use crate::{ - collections::BTreeMap, local_chain::CheckPoint, ConfirmationTimeHeightAnchor, IndexSpk, TxGraph, + collections::BTreeMap, keychain::Indexed, local_chain::CheckPoint, + ConfirmationTimeHeightAnchor, TxGraph, }; use alloc::{boxed::Box, vec::Vec}; use bitcoin::{OutPoint, Script, ScriptBuf, Txid}; @@ -195,7 +196,7 @@ pub struct FullScanRequest { /// [`LocalChain::tip`]: crate::local_chain::LocalChain::tip pub chain_tip: CheckPoint, /// Iterators of script pubkeys indexed by the keychain index. - pub spks_by_keychain: BTreeMap + Send>>, + pub spks_by_keychain: BTreeMap> + Send>>, } impl FullScanRequest { @@ -238,7 +239,7 @@ impl FullScanRequest { pub fn set_spks_for_keychain( mut self, keychain: K, - spks: impl IntoIterator + Send + 'static>, + spks: impl IntoIterator> + Send + 'static>, ) -> Self { self.spks_by_keychain .insert(keychain, Box::new(spks.into_iter())); @@ -252,7 +253,7 @@ impl FullScanRequest { pub fn chain_spks_for_keychain( mut self, keychain: K, - spks: impl IntoIterator + Send + 'static>, + spks: impl IntoIterator> + Send + 'static>, ) -> Self { match self.spks_by_keychain.remove(&keychain) { // clippy here suggests to remove `into_iter` from `spks.into_iter()`, but doing so diff --git a/crates/chain/src/spk_iter.rs b/crates/chain/src/spk_iter.rs index 8285036b..dd4c0a8f 100644 --- a/crates/chain/src/spk_iter.rs +++ b/crates/chain/src/spk_iter.rs @@ -1,5 +1,6 @@ use crate::{ bitcoin::{secp256k1::Secp256k1, ScriptBuf}, + keychain::Indexed, miniscript::{Descriptor, DescriptorPublicKey}, }; use core::{borrow::Borrow, ops::Bound, ops::RangeBounds}; @@ -7,9 +8,6 @@ use core::{borrow::Borrow, ops::Bound, ops::RangeBounds}; /// Maximum [BIP32](https://bips.xyz/32) derivation index. pub const BIP32_MAX_INDEX: u32 = (1 << 31) - 1; -/// A tuple of keychain index and corresponding [`ScriptBuf`]. -pub type IndexSpk = (u32, ScriptBuf); - /// An iterator for derived script pubkeys. /// /// [`SpkIterator`] is an implementation of the [`Iterator`] trait which possesses its own `next()` @@ -100,7 +98,7 @@ impl Iterator for SpkIterator where D: Borrow>, { - type Item = IndexSpk; + type Item = Indexed; fn next(&mut self) -> Option { // For non-wildcard descriptors, we expect the first element to be Some((0, spk)), then None after. diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index c5fd974c..2c1c34e7 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -408,10 +408,10 @@ fn test_wildcard_derivations() { // - next_unused() == ((0, ), 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())); + assert_eq!(spk, (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())); + assert_eq!(spk, (0_u32, external_spk_0.clone())); assert_eq!(&changeset.last_revealed, &[].into()); // - derived till 25 @@ -431,12 +431,12 @@ 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())); + assert_eq!(spk, (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())); + assert_eq!(spk, (16, external_spk_16)); assert_eq!(&changeset.last_revealed, &[].into()); // - Use all the derived till 26. @@ -446,7 +446,7 @@ fn test_wildcard_derivations() { }); let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap(); - assert_eq!(spk, (27, external_spk_27.as_script())); + assert_eq!(spk, (27, external_spk_27)); assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 27)].into()); } @@ -479,7 +479,7 @@ fn test_non_wildcard_derivations() { let (spk, changeset) = txout_index .reveal_next_spk(&TestKeychain::External) .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + assert_eq!(spk, (0, external_spk.clone())); assert_eq!( &changeset.last_revealed, &[(no_wildcard_descriptor.descriptor_id(), 0)].into() @@ -488,7 +488,7 @@ fn test_non_wildcard_derivations() { let (spk, changeset) = txout_index .next_unused_spk(&TestKeychain::External) .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + assert_eq!(spk, (0, external_spk.clone())); assert_eq!(&changeset.last_revealed, &[].into()); // given: @@ -506,13 +506,13 @@ fn test_non_wildcard_derivations() { let (spk, changeset) = txout_index .reveal_next_spk(&TestKeychain::External) .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + assert_eq!(spk, (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())); + assert_eq!(spk, (0, external_spk.clone())); assert_eq!(&changeset.last_revealed, &[].into()); let (revealed_spks, revealed_changeset) = txout_index .reveal_to_target(&TestKeychain::External, 200) diff --git a/crates/esplora/src/async_ext.rs b/crates/esplora/src/async_ext.rs index a7dbc24f..dff52fdd 100644 --- a/crates/esplora/src/async_ext.rs +++ b/crates/esplora/src/async_ext.rs @@ -8,7 +8,7 @@ use bdk_chain::{ local_chain::CheckPoint, BlockId, ConfirmationTimeHeightAnchor, TxGraph, }; -use bdk_chain::{Anchor, IndexSpk}; +use bdk_chain::{Anchor, Indexed}; use esplora_client::{Amount, TxStatus}; use futures::{stream::FuturesOrdered, TryStreamExt}; @@ -236,7 +236,7 @@ async fn full_scan_for_index_and_graph( client: &esplora_client::AsyncClient, keychain_spks: BTreeMap< K, - impl IntoIterator + Send> + Send, + impl IntoIterator> + Send> + Send, >, stop_gap: usize, parallel_requests: usize, diff --git a/crates/esplora/src/blocking_ext.rs b/crates/esplora/src/blocking_ext.rs index 0b173c3a..15036a99 100644 --- a/crates/esplora/src/blocking_ext.rs +++ b/crates/esplora/src/blocking_ext.rs @@ -9,7 +9,7 @@ use bdk_chain::{ local_chain::CheckPoint, BlockId, ConfirmationTimeHeightAnchor, TxGraph, }; -use bdk_chain::{Anchor, IndexSpk}; +use bdk_chain::{Anchor, Indexed}; use esplora_client::TxStatus; use crate::anchor_from_status; @@ -217,7 +217,7 @@ fn chain_update( /// [`KeychainTxOutIndex`](bdk_chain::keychain::KeychainTxOutIndex). fn full_scan_for_index_and_graph_blocking( client: &esplora_client::BlockingClient, - keychain_spks: BTreeMap>, + keychain_spks: BTreeMap>>, stop_gap: usize, parallel_requests: usize, ) -> Result<(TxGraph, BTreeMap), Error> { diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 18498412..33244ca6 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -29,7 +29,7 @@ use bdk_chain::{ spk_client::{FullScanRequest, FullScanResult, SyncRequest, SyncResult}, tx_graph::{CanonicalTx, TxGraph}, Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeHeightAnchor, FullTxOut, - IndexSpk, IndexedTxGraph, + Indexed, IndexedTxGraph, }; use bdk_persist::{Persist, PersistBackend}; use bitcoin::secp256k1::{All, Secp256k1}; @@ -752,7 +752,8 @@ impl Wallet { Ok(AddressInfo { index, - address: Address::from_script(spk, self.network).expect("must have address form"), + address: Address::from_script(spk.as_script(), self.network) + .expect("must have address form"), keychain, }) } @@ -809,7 +810,8 @@ impl Wallet { Ok(AddressInfo { index, - address: Address::from_script(spk, self.network).expect("must have address form"), + address: Address::from_script(spk.as_script(), self.network) + .expect("must have address form"), keychain, }) } @@ -910,7 +912,7 @@ impl Wallet { /// script pubkeys the wallet is storing internally). pub fn all_unbounded_spk_iters( &self, - ) -> BTreeMap + Clone> { + ) -> BTreeMap> + Clone> { self.indexed_graph.index.all_unbounded_spk_iters() } @@ -922,7 +924,7 @@ impl Wallet { pub fn unbounded_spk_iter( &self, keychain: KeychainKind, - ) -> impl Iterator + Clone { + ) -> impl Iterator> + Clone { self.indexed_graph .index .unbounded_spk_iter(&keychain) @@ -932,7 +934,7 @@ impl Wallet { /// Returns the utxo owned by this wallet corresponding to `outpoint` if it exists in the /// wallet's database. pub fn get_utxo(&self, op: OutPoint) -> Option { - let (&(keychain, index), _) = self.indexed_graph.index.txout(op)?; + let ((keychain, index), _) = self.indexed_graph.index.txout(op)?; self.indexed_graph .graph() .filter_chain_unspents( @@ -1511,7 +1513,6 @@ impl Wallet { .index .next_unused_spk(&change_keychain) .expect("keychain must exist"); - let spk = spk.into(); self.indexed_graph.index.mark_used(change_keychain, index); self.persist .stage(ChangeSet::from(indexed_tx_graph::ChangeSet::from( diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 993bde41..3c69a506 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -265,9 +265,6 @@ where .expect("Must exist"); changeset.append(change_changeset); - // Clone to drop the immutable reference. - let change_script = change_script.into(); - let change_plan = bdk_tmp_plan::plan_satisfaction( &graph .index @@ -481,8 +478,8 @@ where local_chain::ChangeSet::default(), indexed_tx_graph::ChangeSet::from(index_changeset), )))?; - let addr = - Address::from_script(spk, network).context("failed to derive address")?; + let addr = Address::from_script(spk.as_script(), network) + .context("failed to derive address")?; println!("[address @ {}] {}", spk_i, addr); Ok(()) } diff --git a/example-crates/example_electrum/src/main.rs b/example-crates/example_electrum/src/main.rs index 175b7ce5..38443e14 100644 --- a/example-crates/example_electrum/src/main.rs +++ b/example-crates/example_electrum/src/main.rs @@ -228,7 +228,7 @@ fn main() -> anyhow::Result<()> { let all_spks = graph .index .revealed_spks(..) - .map(|(index, spk)| (index.to_owned(), spk.to_owned())) + .map(|(index, spk)| (index, spk.to_owned())) .collect::>(); request = request.chain_spks(all_spks.into_iter().map(|((k, spk_i), spk)| { eprint!("Scanning {}: {}", k, spk_i); @@ -239,7 +239,7 @@ fn main() -> anyhow::Result<()> { let unused_spks = graph .index .unused_spks() - .map(|(index, spk)| (index.to_owned(), spk.to_owned())) + .map(|(index, spk)| (index, spk.to_owned())) .collect::>(); request = request.chain_spks(unused_spks.into_iter().map(move |((k, spk_i), spk)| { diff --git a/example-crates/example_esplora/src/main.rs b/example-crates/example_esplora/src/main.rs index d0b16726..3e605a03 100644 --- a/example-crates/example_esplora/src/main.rs +++ b/example-crates/example_esplora/src/main.rs @@ -245,7 +245,7 @@ fn main() -> anyhow::Result<()> { let all_spks = graph .index .revealed_spks(..) - .map(|((k, i), spk)| (k.to_owned(), *i, spk.to_owned())) + .map(|((k, i), spk)| (k, i, spk.to_owned())) .collect::>(); request = request.chain_spks(all_spks.into_iter().map(|(k, i, spk)| { eprint!("scanning {}:{}", k, i); @@ -258,7 +258,7 @@ fn main() -> anyhow::Result<()> { let unused_spks = graph .index .unused_spks() - .map(|(index, spk)| (index.to_owned(), spk.to_owned())) + .map(|(index, spk)| (index, spk.to_owned())) .collect::>(); request = request.chain_spks(unused_spks.into_iter().map(move |((k, i), spk)| { From b8ba5a02066fad7ab2ce276ba071385cd1dbbe3a Mon Sep 17 00:00:00 2001 From: LLFourn Date: Fri, 7 Jun 2024 16:08:59 +1000 Subject: [PATCH 06/14] chore(chain): Improve documentation of keychain::ChangeSet --- crates/chain/src/keychain/txout_index.rs | 208 ++++++++++++----------- 1 file changed, 108 insertions(+), 100 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index f62c2f16..64d65c0b 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -15,87 +15,6 @@ use core::{ use super::*; use crate::Append; -/// Represents updates to the derivation index of a [`KeychainTxOutIndex`]. -/// It maps each keychain `K` to a descriptor and its last revealed index. -/// -/// It can be applied to [`KeychainTxOutIndex`] with [`apply_changeset`]. [`ChangeSet] are -/// monotone in that they will never decrease the revealed derivation index. -/// -/// [`KeychainTxOutIndex`]: crate::keychain::KeychainTxOutIndex -/// [`apply_changeset`]: crate::keychain::KeychainTxOutIndex::apply_changeset -#[derive(Clone, Debug, PartialEq)] -#[cfg_attr( - feature = "serde", - derive(serde::Deserialize, serde::Serialize), - serde( - crate = "serde_crate", - bound( - deserialize = "K: Ord + serde::Deserialize<'de>", - serialize = "K: Ord + serde::Serialize" - ) - ) -)] -#[must_use] -pub struct ChangeSet { - /// Contains the keychains that have been added and their respective descriptor - pub keychains_added: BTreeMap>, - /// Contains for each descriptor_id the last revealed index of derivation - pub last_revealed: BTreeMap, -} - -impl Append for ChangeSet { - /// Append another [`ChangeSet`] into self. - /// - /// For each keychain in `keychains_added` in the given [`ChangeSet`]: - /// If the keychain already exist with a different descriptor, we overwrite the old descriptor. - /// - /// For each `last_revealed` in the given [`ChangeSet`]: - /// If the keychain already exists, increase the index when the other's index > self's index. - fn append(&mut self, other: Self) { - for (new_keychain, new_descriptor) in other.keychains_added { - if !self.keychains_added.contains_key(&new_keychain) - // FIXME: very inefficient - && self - .keychains_added - .values() - .all(|descriptor| descriptor != &new_descriptor) - { - self.keychains_added.insert(new_keychain, new_descriptor); - } - } - - // for `last_revealed`, entries of `other` will take precedence ONLY if it is greater than - // what was originally in `self`. - for (desc_id, index) in other.last_revealed { - use crate::collections::btree_map::Entry; - match self.last_revealed.entry(desc_id) { - Entry::Vacant(entry) => { - entry.insert(index); - } - Entry::Occupied(mut entry) => { - if *entry.get() < index { - entry.insert(index); - } - } - } - } - } - - /// Returns whether the changeset are empty. - fn is_empty(&self) -> bool { - self.last_revealed.is_empty() && self.keychains_added.is_empty() - } -} - -impl Default for ChangeSet { - fn default() -> Self { - Self { - last_revealed: BTreeMap::default(), - keychains_added: BTreeMap::default(), - } - } -} - /// The default lookahead for a [`KeychainTxOutIndex`] pub const DEFAULT_LOOKAHEAD: u32 = 25; @@ -106,6 +25,10 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; /// are identified using the `K` generic. Script pubkeys are identified by the keychain that they /// are derived from `K`, as well as the derivation index `u32`. /// +/// There is a strict 1-to-1 relationship between descriptors and keychains. Each keychain has one +/// and only one descriptor and each descriptor has one and only one keychain. The +/// [`insert_descriptor`] method will return an error if you try and violate this invariant. +/// /// # Revealed script pubkeys /// /// Tracking how script pubkeys are revealed is useful for collecting chain data. For example, if @@ -177,18 +100,19 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; /// [`Ord`]: core::cmp::Ord /// [`SpkTxOutIndex`]: crate::spk_txout_index::SpkTxOutIndex /// [`Descriptor`]: crate::miniscript::Descriptor -/// [`reveal_to_target`]: KeychainTxOutIndex::reveal_to_target -/// [`reveal_next_spk`]: KeychainTxOutIndex::reveal_next_spk -/// [`revealed_keychain_spks`]: KeychainTxOutIndex::revealed_keychain_spks -/// [`revealed_spks`]: KeychainTxOutIndex::revealed_spks -/// [`index_tx`]: KeychainTxOutIndex::index_tx -/// [`index_txout`]: KeychainTxOutIndex::index_txout -/// [`new`]: KeychainTxOutIndex::new -/// [`unbounded_spk_iter`]: KeychainTxOutIndex::unbounded_spk_iter -/// [`all_unbounded_spk_iters`]: KeychainTxOutIndex::all_unbounded_spk_iters -/// [`outpoints`]: KeychainTxOutIndex::outpoints -/// [`txouts`]: KeychainTxOutIndex::txouts -/// [`unused_spks`]: KeychainTxOutIndex::unused_spks +/// [`reveal_to_target`]: Self::reveal_to_target +/// [`reveal_next_spk`]: Self::reveal_next_spk +/// [`revealed_keychain_spks`]: Self::revealed_keychain_spks +/// [`revealed_spks`]: Self::revealed_spks +/// [`index_tx`]: Self::index_tx +/// [`index_txout`]: Self::index_txout +/// [`new`]: Self::new +/// [`unbounded_spk_iter`]: Self::unbounded_spk_iter +/// [`all_unbounded_spk_iters`]: Self::all_unbounded_spk_iters +/// [`outpoints`]: Self::outpoints +/// [`txouts`]: Self::txouts +/// [`unused_spks`]: Self::unused_spks +/// [`insert_descriptor`]: Self::insert_descriptor #[derive(Clone, Debug)] pub struct KeychainTxOutIndex { inner: SpkTxOutIndex<(K, u32)>, @@ -879,19 +803,18 @@ impl KeychainTxOutIndex { .collect() } - /// Applies the derivation changeset to the [`KeychainTxOutIndex`], as specified in the - /// [`ChangeSet::append`] documentation: - /// - Extends the number of derived scripts per keychain - /// - Adds new descriptors introduced - /// - If a descriptor is introduced for a keychain that already had a descriptor, overwrites - /// the old descriptor + /// Applies the `ChangeSet` to the [`KeychainTxOutIndex`] + /// + /// Keychains added by the `keychains_added` field of `ChangeSet` respect the one-to-one + /// keychain <-> descriptor invariant by silently ignoring attempts to violate it (but will + /// panic if `debug_assertions` are enabled). pub fn apply_changeset(&mut self, changeset: ChangeSet) { let ChangeSet { keychains_added, last_revealed, } = changeset; for (keychain, descriptor) in keychains_added { - let _ = self.insert_descriptor(keychain, descriptor); + let _ignore_invariant_violation = self.insert_descriptor(keychain, descriptor); } for (&desc_id, &index) in &last_revealed { @@ -951,3 +874,88 @@ impl core::fmt::Display for InsertDescriptorError { #[cfg(feature = "std")] impl std::error::Error for InsertDescriptorError {} + +/// Represents updates to the derivation index of a [`KeychainTxOutIndex`]. +/// It maps each keychain `K` to a descriptor and its last revealed index. +/// +/// It can be applied to [`KeychainTxOutIndex`] with [`apply_changeset`]. +/// +/// The `last_revealed` field is monotone in that [`append`] will never decrease it. +/// `keychains_added` is *not* monotone, once it is set any attempt to change it is subject to the +/// same *one-to-one* keychain <-> descriptor mapping invariant as [`KeychainTxOutIndex`] itself. +/// +/// [`KeychainTxOutIndex`]: crate::keychain::KeychainTxOutIndex +/// [`apply_changeset`]: crate::keychain::KeychainTxOutIndex::apply_changeset +/// [`append`]: Self::append +#[derive(Clone, Debug, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(serde::Deserialize, serde::Serialize), + serde( + crate = "serde_crate", + bound( + deserialize = "K: Ord + serde::Deserialize<'de>", + serialize = "K: Ord + serde::Serialize" + ) + ) +)] +#[must_use] +pub struct ChangeSet { + /// Contains the keychains that have been added and their respective descriptor + pub keychains_added: BTreeMap>, + /// Contains for each descriptor_id the last revealed index of derivation + pub last_revealed: BTreeMap, +} + +impl Append for ChangeSet { + /// Merge another [`ChangeSet`] into self. + /// + /// For the `keychains_added` field this method respects the invariants of + /// [`insert_descriptor`]. `last_revealed` always becomes the larger of the two. + /// + /// [`insert_descriptor`]: KeychainTxOutIndex::insert_descriptor + fn append(&mut self, other: Self) { + for (new_keychain, new_descriptor) in other.keychains_added { + // enforce 1-to-1 invariance + if !self.keychains_added.contains_key(&new_keychain) + // FIXME: very inefficient + && self + .keychains_added + .values() + .all(|descriptor| descriptor != &new_descriptor) + { + self.keychains_added.insert(new_keychain, new_descriptor); + } + } + + // for `last_revealed`, entries of `other` will take precedence ONLY if it is greater than + // what was originally in `self`. + for (desc_id, index) in other.last_revealed { + use crate::collections::btree_map::Entry; + match self.last_revealed.entry(desc_id) { + Entry::Vacant(entry) => { + entry.insert(index); + } + Entry::Occupied(mut entry) => { + if *entry.get() < index { + entry.insert(index); + } + } + } + } + } + + /// Returns whether the changeset are empty. + fn is_empty(&self) -> bool { + self.last_revealed.is_empty() && self.keychains_added.is_empty() + } +} + +impl Default for ChangeSet { + fn default() -> Self { + Self { + last_revealed: BTreeMap::default(), + keychains_added: BTreeMap::default(), + } + } +} From 5a584d0fd8c138757a10c7af93ec9e09523317e1 Mon Sep 17 00:00:00 2001 From: Lloyd Fournier Date: Tue, 11 Jun 2024 11:12:51 +1000 Subject: [PATCH 07/14] chore(chain): Fix Indexed and KeychainIndexed documentaion Co-authored-by: ValuedMammal --- crates/chain/src/keychain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/chain/src/keychain.rs b/crates/chain/src/keychain.rs index e1e6de68..2a0500a4 100644 --- a/crates/chain/src/keychain.rs +++ b/crates/chain/src/keychain.rs @@ -49,9 +49,9 @@ impl Balance { } } -/// A tuple of keychain index and corresponding [`ScriptBuf`]. +/// A tuple of keychain index and `T` representing the indexed value. pub type Indexed = (u32, T); -/// A tuple of keychain, index and the [`ScriptBuf`] derived at that location. +/// A tuple of keychain `K`, derivation index (`u32`) and a `T` associated with them. pub type KeychainIndexed = ((K, u32), T); impl core::fmt::Display for Balance { From 69f2a695f7dc25478489080598fea0813ea7d93d Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 11 Jun 2024 11:48:32 +1000 Subject: [PATCH 08/14] refactor(chain): improve replenish lookeahd internals see: https://github.com/bitcoindevkit/bdk/pull/1463/files/4eb1e288a9362803b034590e0c56c8cf9cf8b0c2#r1630943639 --- crates/chain/src/keychain/txout_index.rs | 67 +++++++++++++----------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 64d65c0b..65dae939 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -142,15 +142,15 @@ impl Indexer for KeychainTxOutIndex { fn index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet { let mut changeset = ChangeSet::default(); - if let Some((keychain, index)) = self.inner.scan_txout(outpoint, txout) { + if let Some((keychain, index)) = self.inner.scan_txout(outpoint, txout).cloned() { let did = self .keychains_to_descriptor_ids - .get(keychain) + .get(&keychain) .expect("invariant"); - if self.last_revealed.get(did) < Some(index) { - self.last_revealed.insert(*did, *index); - changeset.last_revealed.insert(*did, *index); - self.replenish_lookahead_did(*did); + if self.last_revealed.get(did) < Some(&index) { + self.last_revealed.insert(*did, index); + changeset.last_revealed.insert(*did, index); + self.replenish_lookahead(*did, &keychain, self.lookahead); } } changeset @@ -376,7 +376,7 @@ impl KeychainTxOutIndex { .insert(keychain.clone(), desc_id); self.descriptor_ids_to_keychains .insert(desc_id, keychain.clone()); - self.replenish_lookahead(&keychain, self.lookahead); + self.replenish_lookahead_keychain(&keychain, self.lookahead); changeset .keychains_added .insert(keychain.clone(), descriptor); @@ -439,39 +439,42 @@ impl KeychainTxOutIndex { .filter(|&index| index > 0); if let Some(temp_lookahead) = temp_lookahead { - self.replenish_lookahead(keychain, temp_lookahead); + self.replenish_lookahead_keychain(keychain, temp_lookahead); } } } - fn replenish_lookahead_did(&mut self, did: DescriptorId) { + fn replenish_lookahead_did(&mut self, did: DescriptorId, lookahead: u32) { if let Some(keychain) = self.descriptor_ids_to_keychains.get(&did).cloned() { - self.replenish_lookahead(&keychain, self.lookahead); + self.replenish_lookahead(did, &keychain, lookahead); } } - fn replenish_lookahead(&mut self, keychain: &K, lookahead: u32) { + fn replenish_lookahead_keychain(&mut self, keychain: &K, lookahead: u32) { if let Some(did) = self.keychains_to_descriptor_ids.get(keychain) { - let descriptor = self - .descriptor_ids_to_descriptors - .get(did) - .expect("invariant"); - let next_store_index = self + self.replenish_lookahead(*did, keychain, lookahead); + } + } + + fn replenish_lookahead(&mut self, did: DescriptorId, keychain: &K, lookahead: u32) { + let descriptor = self + .descriptor_ids_to_descriptors + .get(&did) + .expect("invariant"); + let next_store_index = self + .inner + .all_spks() + .range(&(keychain.clone(), u32::MIN)..=&(keychain.clone(), u32::MAX)) + .last() + .map_or(0, |((_, index), _)| *index + 1); + let next_reveal_index = self.last_revealed.get(&did).map_or(0, |v| *v + 1); + for (new_index, new_spk) in + SpkIterator::new_with_range(descriptor, next_store_index..next_reveal_index + lookahead) + { + let _inserted = self .inner - .all_spks() - .range(&(keychain.clone(), u32::MIN)..=&(keychain.clone(), u32::MAX)) - .last() - .map_or(0, |((_, index), _)| *index + 1); - let next_reveal_index = self.last_revealed.get(did).map_or(0, |v| *v + 1); - for (new_index, new_spk) in SpkIterator::new_with_range( - descriptor, - next_store_index..next_reveal_index + lookahead, - ) { - let _inserted = self - .inner - .insert_spk((keychain.clone(), new_index), new_spk); - debug_assert!(_inserted, "replenish lookahead: must not have existing spk: keychain={:?}, lookahead={}, next_store_index={}, next_reveal_index={}", keychain, lookahead, next_store_index, next_reveal_index); - } + .insert_spk((keychain.clone(), new_index), new_spk); + debug_assert!(_inserted, "replenish lookahead: must not have existing spk: keychain={:?}, lookahead={}, next_store_index={}, next_reveal_index={}", keychain, lookahead, next_store_index, next_reveal_index); } } @@ -719,7 +722,7 @@ impl KeychainTxOutIndex { let _ = self.inner.insert_spk((keychain.clone(), next_index), spk); self.last_revealed.insert(*did, next_index); changeset.last_revealed.insert(*did, next_index); - self.replenish_lookahead(keychain, self.lookahead); + self.replenish_lookahead_keychain(keychain, self.lookahead); } let script = self .inner @@ -823,7 +826,7 @@ impl KeychainTxOutIndex { } for did in last_revealed.keys() { - self.replenish_lookahead_did(*did); + self.replenish_lookahead_did(*did, self.lookahead); } } } From 8779afdb0bf4e9b1004f47f86a770d25938d206d Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 11 Jun 2024 14:29:26 +1000 Subject: [PATCH 09/14] chore(chain): document insert_descriptor invariants better --- crates/chain/src/keychain/txout_index.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 65dae939..d47aa843 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -27,7 +27,11 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; /// /// There is a strict 1-to-1 relationship between descriptors and keychains. Each keychain has one /// and only one descriptor and each descriptor has one and only one keychain. The -/// [`insert_descriptor`] method will return an error if you try and violate this invariant. +/// [`insert_descriptor`] method will return an error if you try and violate this invariant. This +/// rule is a proxy for a stronger rule: no two descriptors should produce the same script pubkey. +/// Having two descriptors produce the same script pubkey should cause whichever keychain derives the +/// script pubkey first to be the effective owner of it but you should not rely on this behaviour. +/// ⚠ It is up you, the developer, not to violate this invariant. /// /// # Revealed script pubkeys /// From 4d3846abf4f59b4a97bb825281655a6b67275603 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 11 Jun 2024 14:55:06 +1000 Subject: [PATCH 10/14] chore(chain): s/replenish_lookahead/replenish_inner_index/ --- crates/chain/src/keychain/txout_index.rs | 54 +++++++++++++----------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index d47aa843..252be639 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -154,7 +154,7 @@ impl Indexer for KeychainTxOutIndex { if self.last_revealed.get(did) < Some(&index) { self.last_revealed.insert(*did, index); changeset.last_revealed.insert(*did, index); - self.replenish_lookahead(*did, &keychain, self.lookahead); + self.replenish_inner_index(*did, &keychain, self.lookahead); } } changeset @@ -370,17 +370,17 @@ impl KeychainTxOutIndex { descriptor: Descriptor, ) -> Result, InsertDescriptorError> { let mut changeset = ChangeSet::::default(); - let desc_id = descriptor.descriptor_id(); + let did = descriptor.descriptor_id(); if !self.keychains_to_descriptor_ids.contains_key(&keychain) - && !self.descriptor_ids_to_keychains.contains_key(&desc_id) + && !self.descriptor_ids_to_keychains.contains_key(&did) { self.descriptor_ids_to_descriptors - .insert(desc_id, descriptor.clone()); + .insert(did, descriptor.clone()); self.keychains_to_descriptor_ids - .insert(keychain.clone(), desc_id); + .insert(keychain.clone(), did); self.descriptor_ids_to_keychains - .insert(desc_id, keychain.clone()); - self.replenish_lookahead_keychain(&keychain, self.lookahead); + .insert(did, keychain.clone()); + self.replenish_inner_index(did, &keychain, self.lookahead); changeset .keychains_added .insert(keychain.clone(), descriptor); @@ -390,7 +390,7 @@ impl KeychainTxOutIndex { .descriptor_ids_to_descriptors .get(existing_desc_id) .expect("invariant"); - if *existing_desc_id != desc_id { + if *existing_desc_id != did { return Err(InsertDescriptorError::KeychainAlreadyAssigned { existing_assignment: descriptor.clone(), keychain, @@ -398,10 +398,10 @@ impl KeychainTxOutIndex { } } - if let Some(existing_keychain) = self.descriptor_ids_to_keychains.get(&desc_id) { + if let Some(existing_keychain) = self.descriptor_ids_to_keychains.get(&did) { let descriptor = self .descriptor_ids_to_descriptors - .get(&desc_id) + .get(&did) .expect("invariant") .clone(); @@ -443,24 +443,25 @@ impl KeychainTxOutIndex { .filter(|&index| index > 0); if let Some(temp_lookahead) = temp_lookahead { - self.replenish_lookahead_keychain(keychain, temp_lookahead); + self.replenish_inner_index_keychain(keychain, temp_lookahead); } } } - fn replenish_lookahead_did(&mut self, did: DescriptorId, lookahead: u32) { + fn replenish_inner_index_did(&mut self, did: DescriptorId, lookahead: u32) { if let Some(keychain) = self.descriptor_ids_to_keychains.get(&did).cloned() { - self.replenish_lookahead(did, &keychain, lookahead); + self.replenish_inner_index(did, &keychain, lookahead); } } - fn replenish_lookahead_keychain(&mut self, keychain: &K, lookahead: u32) { + fn replenish_inner_index_keychain(&mut self, keychain: &K, lookahead: u32) { if let Some(did) = self.keychains_to_descriptor_ids.get(keychain) { - self.replenish_lookahead(*did, keychain, lookahead); + self.replenish_inner_index(*did, keychain, lookahead); } } - fn replenish_lookahead(&mut self, did: DescriptorId, keychain: &K, lookahead: u32) { + /// Syncs the state of the inner spk index after changes to a keychain + fn replenish_inner_index(&mut self, did: DescriptorId, keychain: &K, lookahead: u32) { let descriptor = self .descriptor_ids_to_descriptors .get(&did) @@ -677,6 +678,7 @@ impl KeychainTxOutIndex { /// pubkeys are revealed, then both of these will be empty. /// /// Returns None if the provided `keychain` doesn't exist. + #[must_use] pub fn reveal_to_target( &mut self, keychain: &K, @@ -718,15 +720,9 @@ impl KeychainTxOutIndex { if new { let did = self.keychains_to_descriptor_ids.get(keychain)?; - let descriptor = self.descriptor_ids_to_descriptors.get(did)?; - let spk = descriptor - .at_derivation_index(next_index) - .expect("already checked index is not too high") - .script_pubkey(); - let _ = self.inner.insert_spk((keychain.clone(), next_index), spk); self.last_revealed.insert(*did, next_index); changeset.last_revealed.insert(*did, next_index); - self.replenish_lookahead_keychain(keychain, self.lookahead); + self.replenish_inner_index(*did, keychain, self.lookahead); } let script = self .inner @@ -830,7 +826,7 @@ impl KeychainTxOutIndex { } for did in last_revealed.keys() { - self.replenish_lookahead_did(*did, self.lookahead); + self.replenish_inner_index_did(*did, self.lookahead); } } } @@ -966,3 +962,13 @@ impl Default for ChangeSet { } } } + +#[derive(Clone, Debug, Eq, PartialEq)] +/// The keychain doesn't exist. Most likley hasn't been inserted with [`KeychainTxOutIndex::insert_descriptor`]. +pub struct NoSuchKeychain(K); + +impl core::fmt::Display for NoSuchKeychain { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "no such keychain {:?} exists", &self.0) + } +} From c77e12bae7f465ec7fb08b8be16d99793c757cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 12 Jun 2024 19:10:52 +0800 Subject: [PATCH 11/14] refactor(chain): `KeychainTxOutIndex` use `HashMap` for fields Instead of `BTreeMap` which is less performant. --- crates/chain/src/keychain/txout_index.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 252be639..b8c0c457 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -123,12 +123,12 @@ pub struct KeychainTxOutIndex { /// keychain -> (descriptor id) map keychains_to_descriptor_ids: BTreeMap, /// descriptor id -> keychain map - descriptor_ids_to_keychains: BTreeMap, + descriptor_ids_to_keychains: HashMap, /// descriptor_id -> descriptor map /// This is a "monotone" map, meaning that its size keeps growing, i.e., we never delete /// descriptors from it. This is useful for revealing spks for descriptors that don't have /// keychains associated. - descriptor_ids_to_descriptors: BTreeMap>, + descriptor_ids_to_descriptors: HashMap>, /// last revealed indices for each descriptor. last_revealed: HashMap, /// lookahead setting @@ -201,8 +201,8 @@ impl KeychainTxOutIndex { pub fn new(lookahead: u32) -> Self { Self { inner: SpkTxOutIndex::default(), - keychains_to_descriptor_ids: BTreeMap::new(), - descriptor_ids_to_descriptors: BTreeMap::new(), + keychains_to_descriptor_ids: Default::default(), + descriptor_ids_to_descriptors: Default::default(), descriptor_ids_to_keychains: Default::default(), last_revealed: Default::default(), lookahead, From 5a02f40122f1bfa06c80bac93f68f5799225d133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 12 Jun 2024 21:50:03 +0800 Subject: [PATCH 12/14] docs(chain): fix docs --- crates/chain/src/keychain/txout_index.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index b8c0c457..b43b7376 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -29,9 +29,9 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; /// and only one descriptor and each descriptor has one and only one keychain. The /// [`insert_descriptor`] method will return an error if you try and violate this invariant. This /// rule is a proxy for a stronger rule: no two descriptors should produce the same script pubkey. -/// Having two descriptors produce the same script pubkey should cause whichever keychain derives the -/// script pubkey first to be the effective owner of it but you should not rely on this behaviour. -/// ⚠ It is up you, the developer, not to violate this invariant. +/// Having two descriptors produce the same script pubkey should cause whichever keychain derives +/// the script pubkey first to be the effective owner of it but you should not rely on this +/// behaviour. ⚠ It is up you, the developer, not to violate this invariant. /// /// # Revealed script pubkeys /// @@ -341,7 +341,7 @@ impl KeychainTxOutIndex { } impl KeychainTxOutIndex { - /// Return the map of the keychain to descriptors. + /// Return all keychains and their corresponding descriptors. pub fn keychains( &self, ) -> impl DoubleEndedIterator)> + ExactSizeIterator + '_ From 639d735ca0ae54d8b2c3bc28241032154b94d45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 12 Jun 2024 22:19:54 +0800 Subject: [PATCH 13/14] refactor(chain): change field names to be more sane --- crates/chain/src/keychain/txout_index.rs | 112 ++++++++--------------- 1 file changed, 38 insertions(+), 74 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index b43b7376..0bdd6c94 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -120,18 +120,10 @@ pub const DEFAULT_LOOKAHEAD: u32 = 25; #[derive(Clone, Debug)] pub struct KeychainTxOutIndex { inner: SpkTxOutIndex<(K, u32)>, - /// keychain -> (descriptor id) map - keychains_to_descriptor_ids: BTreeMap, - /// descriptor id -> keychain map - descriptor_ids_to_keychains: HashMap, - /// descriptor_id -> descriptor map - /// This is a "monotone" map, meaning that its size keeps growing, i.e., we never delete - /// descriptors from it. This is useful for revealing spks for descriptors that don't have - /// keychains associated. - descriptor_ids_to_descriptors: HashMap>, - /// last revealed indices for each descriptor. + keychain_to_descriptor_id: BTreeMap, + descriptor_id_to_keychain: HashMap, + descriptors: HashMap>, last_revealed: HashMap, - /// lookahead setting lookahead: u32, } @@ -148,7 +140,7 @@ impl Indexer for KeychainTxOutIndex { let mut changeset = ChangeSet::default(); if let Some((keychain, index)) = self.inner.scan_txout(outpoint, txout).cloned() { let did = self - .keychains_to_descriptor_ids + .keychain_to_descriptor_id .get(&keychain) .expect("invariant"); if self.last_revealed.get(did) < Some(&index) { @@ -201,9 +193,9 @@ impl KeychainTxOutIndex { pub fn new(lookahead: u32) -> Self { Self { inner: SpkTxOutIndex::default(), - keychains_to_descriptor_ids: Default::default(), - descriptor_ids_to_descriptors: Default::default(), - descriptor_ids_to_keychains: Default::default(), + keychain_to_descriptor_id: Default::default(), + descriptors: Default::default(), + descriptor_id_to_keychain: Default::default(), last_revealed: Default::default(), lookahead, } @@ -346,14 +338,9 @@ impl KeychainTxOutIndex { &self, ) -> impl DoubleEndedIterator)> + ExactSizeIterator + '_ { - self.keychains_to_descriptor_ids.iter().map(|(k, did)| { - ( - k, - self.descriptor_ids_to_descriptors - .get(did) - .expect("invariant"), - ) - }) + self.keychain_to_descriptor_id + .iter() + .map(|(k, did)| (k, self.descriptors.get(did).expect("invariant"))) } /// Insert a descriptor with a keychain associated to it. @@ -371,25 +358,19 @@ impl KeychainTxOutIndex { ) -> Result, InsertDescriptorError> { let mut changeset = ChangeSet::::default(); let did = descriptor.descriptor_id(); - if !self.keychains_to_descriptor_ids.contains_key(&keychain) - && !self.descriptor_ids_to_keychains.contains_key(&did) + if !self.keychain_to_descriptor_id.contains_key(&keychain) + && !self.descriptor_id_to_keychain.contains_key(&did) { - self.descriptor_ids_to_descriptors - .insert(did, descriptor.clone()); - self.keychains_to_descriptor_ids - .insert(keychain.clone(), did); - self.descriptor_ids_to_keychains - .insert(did, keychain.clone()); + self.descriptors.insert(did, descriptor.clone()); + self.keychain_to_descriptor_id.insert(keychain.clone(), did); + self.descriptor_id_to_keychain.insert(did, keychain.clone()); self.replenish_inner_index(did, &keychain, self.lookahead); changeset .keychains_added .insert(keychain.clone(), descriptor); } else { - if let Some(existing_desc_id) = self.keychains_to_descriptor_ids.get(&keychain) { - let descriptor = self - .descriptor_ids_to_descriptors - .get(existing_desc_id) - .expect("invariant"); + if let Some(existing_desc_id) = self.keychain_to_descriptor_id.get(&keychain) { + let descriptor = self.descriptors.get(existing_desc_id).expect("invariant"); if *existing_desc_id != did { return Err(InsertDescriptorError::KeychainAlreadyAssigned { existing_assignment: descriptor.clone(), @@ -398,12 +379,8 @@ impl KeychainTxOutIndex { } } - if let Some(existing_keychain) = self.descriptor_ids_to_keychains.get(&did) { - let descriptor = self - .descriptor_ids_to_descriptors - .get(&did) - .expect("invariant") - .clone(); + if let Some(existing_keychain) = self.descriptor_id_to_keychain.get(&did) { + let descriptor = self.descriptors.get(&did).expect("invariant").clone(); if *existing_keychain != keychain { return Err(InsertDescriptorError::DescriptorAlreadyAssigned { @@ -420,8 +397,8 @@ impl KeychainTxOutIndex { /// Gets the descriptor associated with the keychain. Returns `None` if the keychain doesn't /// have a descriptor associated with it. pub fn get_descriptor(&self, keychain: &K) -> Option<&Descriptor> { - let did = self.keychains_to_descriptor_ids.get(keychain)?; - self.descriptor_ids_to_descriptors.get(did) + let did = self.keychain_to_descriptor_id.get(keychain)?; + self.descriptors.get(did) } /// Get the lookahead setting. @@ -449,23 +426,20 @@ impl KeychainTxOutIndex { } fn replenish_inner_index_did(&mut self, did: DescriptorId, lookahead: u32) { - if let Some(keychain) = self.descriptor_ids_to_keychains.get(&did).cloned() { + if let Some(keychain) = self.descriptor_id_to_keychain.get(&did).cloned() { self.replenish_inner_index(did, &keychain, lookahead); } } fn replenish_inner_index_keychain(&mut self, keychain: &K, lookahead: u32) { - if let Some(did) = self.keychains_to_descriptor_ids.get(keychain) { + if let Some(did) = self.keychain_to_descriptor_id.get(keychain) { self.replenish_inner_index(*did, keychain, lookahead); } } /// Syncs the state of the inner spk index after changes to a keychain fn replenish_inner_index(&mut self, did: DescriptorId, keychain: &K, lookahead: u32) { - let descriptor = self - .descriptor_ids_to_descriptors - .get(&did) - .expect("invariant"); + let descriptor = self.descriptors.get(&did).expect("invariant"); let next_store_index = self .inner .all_spks() @@ -497,17 +471,12 @@ impl KeychainTxOutIndex { pub fn all_unbounded_spk_iters( &self, ) -> BTreeMap>> { - self.keychains_to_descriptor_ids + self.keychain_to_descriptor_id .iter() .map(|(k, did)| { ( k.clone(), - SpkIterator::new( - self.descriptor_ids_to_descriptors - .get(did) - .expect("invariant") - .clone(), - ), + SpkIterator::new(self.descriptors.get(did).expect("invariant").clone()), ) }) .collect() @@ -521,7 +490,7 @@ impl KeychainTxOutIndex { let start = range.start_bound(); let end = range.end_bound(); let mut iter_last_revealed = self - .keychains_to_descriptor_ids + .keychain_to_descriptor_id .range((start, end)) .map(|(k, did)| (k, self.last_revealed.get(did).cloned())); let mut iter_spks = self @@ -571,12 +540,10 @@ impl KeychainTxOutIndex { pub fn unused_spks( &self, ) -> impl DoubleEndedIterator> + Clone { - self.keychains_to_descriptor_ids - .keys() - .flat_map(|keychain| { - self.unused_keychain_spks(keychain) - .map(|(i, spk)| ((keychain.clone(), i), spk)) - }) + self.keychain_to_descriptor_id.keys().flat_map(|keychain| { + self.unused_keychain_spks(keychain) + .map(|(i, spk)| ((keychain.clone(), i), spk)) + }) } /// Iterate over revealed, but unused, spks of the given `keychain`. @@ -585,7 +552,7 @@ impl KeychainTxOutIndex { &self, keychain: &K, ) -> impl DoubleEndedIterator> + Clone { - let end = match self.keychains_to_descriptor_ids.get(keychain) { + let end = match self.keychain_to_descriptor_id.get(keychain) { Some(did) => self.last_revealed.get(did).map(|v| *v + 1).unwrap_or(0), None => 0, }; @@ -608,12 +575,9 @@ impl KeychainTxOutIndex { /// /// Returns None if the provided `keychain` doesn't exist. pub fn next_index(&self, keychain: &K) -> Option<(u32, bool)> { - let did = self.keychains_to_descriptor_ids.get(keychain)?; + let did = self.keychain_to_descriptor_id.get(keychain)?; let last_index = self.last_revealed.get(did).cloned(); - let descriptor = self - .descriptor_ids_to_descriptors - .get(did) - .expect("invariant"); + let descriptor = self.descriptors.get(did).expect("invariant"); // we can only get the next index if the wildcard exists. let has_wildcard = descriptor.has_wildcard(); @@ -640,7 +604,7 @@ impl KeychainTxOutIndex { self.last_revealed .iter() .filter_map(|(desc_id, index)| { - let keychain = self.descriptor_ids_to_keychains.get(desc_id)?; + let keychain = self.descriptor_id_to_keychain.get(desc_id)?; Some((keychain.clone(), *index)) }) .collect() @@ -649,7 +613,7 @@ impl KeychainTxOutIndex { /// Get the last derivation index revealed for `keychain`. Returns None if the keychain doesn't /// exist, or if the keychain doesn't have any revealed scripts. pub fn last_revealed_index(&self, keychain: &K) -> Option { - let descriptor_id = self.keychains_to_descriptor_ids.get(keychain)?; + let descriptor_id = self.keychain_to_descriptor_id.get(keychain)?; self.last_revealed.get(descriptor_id).cloned() } @@ -719,7 +683,7 @@ impl KeychainTxOutIndex { let mut changeset = ChangeSet::default(); if new { - let did = self.keychains_to_descriptor_ids.get(keychain)?; + let did = self.keychain_to_descriptor_id.get(keychain)?; self.last_revealed.insert(*did, next_index); changeset.last_revealed.insert(*did, next_index); self.replenish_inner_index(*did, keychain, self.lookahead); @@ -797,7 +761,7 @@ impl KeychainTxOutIndex { /// Returns the highest derivation index of each keychain that [`KeychainTxOutIndex`] has found /// a [`TxOut`] with it's script pubkey. pub fn last_used_indices(&self) -> BTreeMap { - self.keychains_to_descriptor_ids + self.keychain_to_descriptor_id .iter() .filter_map(|(keychain, _)| { self.last_used_index(keychain) From 8dd174479f9719309663ed979a5b4b86aca0a6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 13 Jun 2024 22:56:19 +0800 Subject: [PATCH 14/14] refactor(chain): compute txid once for `KeychainTxOutIndex::index_tx` --- crates/chain/src/keychain/txout_index.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 0bdd6c94..cf0f21f8 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -154,8 +154,9 @@ impl Indexer for KeychainTxOutIndex { fn index_tx(&mut self, tx: &bitcoin::Transaction) -> Self::ChangeSet { let mut changeset = ChangeSet::::default(); + let txid = tx.compute_txid(); for (op, txout) in tx.output.iter().enumerate() { - changeset.append(self.index_txout(OutPoint::new(tx.compute_txid(), op as u32), txout)); + changeset.append(self.index_txout(OutPoint::new(txid, op as u32), txout)); } changeset }