diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 994553b3f..db7ad63fe 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -5,7 +5,10 @@ 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; +use bitcoin::{ + hashes::Hash, Amount, OutPoint, Script, ScriptBuf, SignedAmount, Transaction, TxOut, Txid, +}; use core::{ fmt::Debug, ops::{Bound, RangeBounds}, @@ -127,8 +130,8 @@ const DEFAULT_LOOKAHEAD: u32 = 25; /// /// # Change sets /// -/// Methods that can update the last revealed index or add keychains will return [`super::ChangeSet`] to report -/// these changes. This can be persisted for future recovery. +/// 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 /// @@ -207,14 +210,16 @@ const DEFAULT_LOOKAHEAD: u32 = 25; #[derive(Clone, Debug)] pub struct KeychainTxOutIndex { inner: SpkTxOutIndex<(DescriptorId, 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>, + // keychain -> descriptor_id map + keychains_to_descriptor_ids: BTreeMap, + // descriptor_id -> keychain set + // This is a reverse map of `keychains_to_descriptors`. Although there is only one descriptor + // per keychain, different keychains can refer to the same descriptor, therefore we have a set + // of keychains per descriptor. When associated data (such as spks, outpoints) are returned with + // a keychain, we return it with the highest-ranked keychain with it. We rank keychains by + // `Ord`, therefore the keychain set is a `BTreeSet`. The earliest keychain variant (according + // to `Ord`) has precedence. + 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 @@ -233,23 +238,27 @@ 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"); + let desc = self + .descriptor_ids_to_descriptors + .get(&descriptor_id) + .cloned() + .expect("descriptors are added monotonically, scanned txout descriptor ids must have associated descriptors"); + let (_, changeset) = self.reveal_to_target_with_descriptor(desc, index); changeset } - None => super::ChangeSet::default(), + None => ChangeSet::default(), } } 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.txid(), op as u32), txout)); } @@ -257,7 +266,7 @@ impl Indexer for KeychainTxOutIndex { } fn initial_changeset(&self) -> Self::ChangeSet { - super::ChangeSet { + ChangeSet { keychains_added: self .keychains() .map(|(k, v)| (k.clone(), v.clone())) @@ -289,8 +298,8 @@ 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_keychains: HashMap::new(), descriptor_ids_to_descriptors: BTreeMap::new(), last_revealed: BTreeMap::new(), lookahead, @@ -302,7 +311,7 @@ impl KeychainTxOutIndex { 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)?; + let keychains = self.descriptor_ids_to_keychains.get(desc_id)?; keychains.iter().next() } @@ -362,7 +371,7 @@ impl KeychainTxOutIndex { /// /// 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; + let descriptor_id = *self.keychains_to_descriptor_ids.get(&keychain)?; self.inner.spk_at_index(&(descriptor_id, index)) } @@ -382,7 +391,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); + let descriptor_id = self.keychains_to_descriptor_ids.get(&keychain).copied(); match descriptor_id { Some(descriptor_id) => self.inner.is_used(&(descriptor_id, index)), None => false, @@ -406,7 +415,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); + let descriptor_id = self.keychains_to_descriptor_ids.get(&keychain).copied(); match descriptor_id { Some(descriptor_id) => self.inner.mark_used(&(descriptor_id, index)), None => false, @@ -423,7 +432,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); + let descriptor_id = self.keychains_to_descriptor_ids.get(&keychain).copied(); match descriptor_id { Some(descriptor_id) => self.inner.unmark_used(&(descriptor_id, index)), None => false, @@ -462,9 +471,13 @@ impl KeychainTxOutIndex { &self, ) -> impl DoubleEndedIterator)> + ExactSizeIterator + '_ { - self.keychains_to_descriptors - .iter() - .map(|(k, (_, d))| (k, d)) + self.keychains_to_descriptor_ids.iter().map(|(k, desc_id)| { + let descriptor = self + .descriptor_ids_to_descriptors + .get(desc_id) + .expect("descriptor id cannot be associated with keychain without descriptor"); + (k, descriptor) + }) } /// Insert a descriptor with a keychain associated to it. @@ -479,15 +492,15 @@ impl KeychainTxOutIndex { &mut self, keychain: K, descriptor: Descriptor, - ) -> super::ChangeSet { - let mut changeset = super::ChangeSet::::default(); + ) -> ChangeSet { + 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())); + let old_desc_id = self + .keychains_to_descriptor_ids + .insert(keychain.clone(), desc_id); - if let Some((old_desc_id, _)) = old_desc { + if let Some(old_desc_id) = old_desc_id { // nothing needs to be done if caller reinsterted the same descriptor under the same // keychain if old_desc_id == desc_id { @@ -497,14 +510,14 @@ impl KeychainTxOutIndex { // 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 + .descriptor_ids_to_keychains .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 + self.descriptor_ids_to_keychains .entry(desc_id) .or_default() .insert(keychain.clone()); @@ -521,7 +534,13 @@ 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> { - self.keychains_to_descriptors.get(keychain).map(|(_, d)| d) + self.keychains_to_descriptor_ids + .get(keychain) + .map(|desc_id| { + self.descriptor_ids_to_descriptors + .get(desc_id) + .expect("descriptor id cannot be associated with keychain without descriptor") + }) } /// Get the lookahead setting. @@ -549,8 +568,13 @@ 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 descriptor_id = self.keychains_to_descriptor_ids.get(keychain).copied(); + if let Some(descriptor_id) = descriptor_id { + let descriptor = self + .descriptor_ids_to_descriptors + .get(&descriptor_id) + .expect("descriptor id cannot be associated with keychain without descriptor"); + 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); @@ -580,17 +604,29 @@ impl KeychainTxOutIndex { &self, keychain: &K, ) -> Option>> { - let descriptor = self.keychains_to_descriptors.get(keychain)?.1.clone(); - Some(SpkIterator::new(descriptor)) + let desc_id = self.keychains_to_descriptor_ids.get(keychain)?; + let desc = self + .descriptor_ids_to_descriptors + .get(desc_id) + .cloned() + .expect("descriptor id cannot be associated with keychain without descriptor"); + Some(SpkIterator::new(desc)) } /// Get unbounded spk iterators for all keychains. 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, desc_id)| { + let desc = self + .descriptor_ids_to_descriptors + .get(desc_id) + .cloned() + .expect("descriptor id cannot be associated with keychain without descriptor"); + (k.clone(), SpkIterator::new(desc)) + }) .collect() } @@ -599,9 +635,9 @@ impl KeychainTxOutIndex { &self, range: impl RangeBounds, ) -> impl DoubleEndedIterator + Clone { - self.keychains_to_descriptors + self.keychains_to_descriptor_ids .range(range) - .flat_map(|(_, (descriptor_id, _))| { + .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)), @@ -633,10 +669,12 @@ impl KeychainTxOutIndex { /// 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)) - }) + 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`. @@ -646,9 +684,9 @@ impl KeychainTxOutIndex { keychain: &K, ) -> impl DoubleEndedIterator + Clone { let desc_id = self - .keychains_to_descriptors + .keychains_to_descriptor_ids .get(keychain) - .map(|(desc_id, _)| *desc_id) + .cloned() // 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 @@ -672,25 +710,45 @@ 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 descriptor_id = self.keychains_to_descriptor_ids.get(keychain)?; + let descriptor = self + .descriptor_ids_to_descriptors + .get(descriptor_id) + .expect("descriptor id cannot be associated with keychain without descriptor"); + Some(self.next_index_with_descriptor(descriptor)) + } - // we can only get the next index if the wildcard exists. + /// Get the next derivation index of given `descriptor`. The next derivation index is the index + /// after the last revealed index. + /// + /// The second field of the returned tuple represents whether the next derivation index is new. + /// There are two scenarios where the next derivation index is reused (not new): + /// + /// 1. The keychain's descriptor has no wildcard, and a script has already been revealed. + /// 2. The number of revealed scripts has already reached 2^31 (refer to BIP-32). + /// + /// Not checking the second field of the tuple may result in address reuse. + fn next_index_with_descriptor( + &self, + descriptor: &Descriptor, + ) -> (u32, bool) { + let desc_id = descriptor.descriptor_id(); + + // we can only get the next index if the wildcard exists let has_wildcard = descriptor.has_wildcard(); + let last_index = self.last_revealed.get(&desc_id).cloned(); - Some(match last_index { - // if there is no index, next_index is always 0. + match last_index { + // if there is no index, next_index is always 0 None => (0, true), - // descriptors without wildcards can only have one index. + // descriptors without wildcards can only have one index Some(_) if !has_wildcard => (0, false), - // derivation index must be < 2^31 (BIP-32). - Some(index) if index > BIP32_MAX_INDEX => { - unreachable!("index is out of bounds") - } + // derivation index must be < 2^31 (BIP-32) + Some(index) if index > BIP32_MAX_INDEX => unreachable!("index out of bounds"), Some(index) if index == BIP32_MAX_INDEX => (index, false), - // get the next derivation index. + // get the next derivation index Some(index) => (index + 1, true), - }) + } } /// Get the last derivation index that is revealed for each keychain. @@ -709,8 +767,8 @@ 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. @@ -719,17 +777,16 @@ impl KeychainTxOutIndex { keychains: &BTreeMap, ) -> ( BTreeMap>>, - super::ChangeSet, + ChangeSet, ) { - let mut changeset = super::ChangeSet::default(); + let mut changeset = ChangeSet::default(); let mut spks = BTreeMap::new(); 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()); - } + let (new_spks, new_changeset) = self.reveal_to_target(keychain, index); + if let Some(new_spks) = new_spks { + changeset.append(new_changeset); + spks.insert(keychain.clone(), new_spks); } } @@ -742,18 +799,12 @@ impl KeychainTxOutIndex { /// 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( + fn reveal_to_target_with_descriptor( &mut self, - descriptor_id: DescriptorId, + descriptor: Descriptor, target_index: u32, - ) -> Option<( - SpkIterator>, - super::ChangeSet, - )> { - let descriptor = self - .descriptor_ids_to_descriptors - .get(&descriptor_id)? - .clone(); + ) -> (SpkIterator>, ChangeSet) { + let descriptor_id = descriptor.descriptor_id(); let has_wildcard = descriptor.has_wildcard(); let target_index = if has_wildcard { target_index } else { 0 }; @@ -766,10 +817,10 @@ impl KeychainTxOutIndex { // If the target_index is already revealed, we are done if next_reveal_index > target_index { - return Some(( + return ( SpkIterator::new_with_range(descriptor, next_reveal_index..next_reveal_index), - super::ChangeSet::default(), - )); + ChangeSet::default(), + ); } // We range over the indexes that are not stored and insert their spks in the index. @@ -787,13 +838,13 @@ impl KeychainTxOutIndex { 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 { + ChangeSet { keychains_added: BTreeMap::new(), last_revealed: core::iter::once((descriptor_id, target_index)).collect(), }, - )) + ) } /// Reveals script pubkeys of the `keychain`'s descriptor **up to and including** the @@ -804,76 +855,94 @@ impl KeychainTxOutIndex { /// 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 - /// pubkeys are revealed, then both of these will be empty. + /// [`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. pub fn reveal_to_target( &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>>, + ChangeSet, + ) { + let descriptor_id = match self.keychains_to_descriptor_ids.get(keychain) { + Some(desc_id) => desc_id, + None => return (None, ChangeSet::default()), + }; + let desc = self + .descriptor_ids_to_descriptors + .get(descriptor_id) + .cloned() + .expect("descriptors are added monotonically, scanned txout descriptor ids must have associated descriptors"); + let (spk_iter, changeset) = self.reveal_to_target_with_descriptor(desc, target_index); + (Some(spk_iter), 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). - /// Returns None if the provided keychain doesn't exist. + /// Returns the next revealed script pubkey (alongside it's derivation index), and a + /// [`ChangeSet`]. The next revealed script pubkey is `None` if the provided `keychain` has no + /// associated descriptor. /// - /// 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: + /// When the descriptor has no more script pubkeys to reveal, the last revealed script and an + /// empty [`ChangeSet`] is returned. There are 3 scenarios in which a descriptor can no longer + /// derive scripts: /// /// 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; - let script = self - .inner - .spk_at_index(&(descriptor_id, next_index)) - .expect("script must already be stored"); - Some(((next_index, script), changeset)) + pub fn reveal_next_spk(&mut self, keychain: &K) -> (Option<(u32, ScriptBuf)>, ChangeSet) { + let descriptor_id = match self.keychains_to_descriptor_ids.get(keychain) { + Some(&desc_id) => desc_id, + None => { + return (None, Default::default()); + } + }; + let descriptor = self + .descriptor_ids_to_descriptors + .get(&descriptor_id) + .expect("descriptor id cannot be associated with keychain without descriptor"); + let (next_index, _) = self.next_index_with_descriptor(descriptor); + let (mut new_spks, changeset) = + self.reveal_to_target_with_descriptor(descriptor.clone(), next_index); + let revealed_spk = new_spks.next().unwrap_or_else(|| { + // if we have exhausted all derivation indicies, the returned `next_index` is reused + // and will not be included in `new_spks` (refer to `next_index_with_descriptor` docs) + let spk = self + .inner + .spk_at_index(&(descriptor_id, next_index)) + .expect("script must already be stored") + .to_owned(); + (next_index, spk) + }); + debug_assert_eq!(new_spks.next(), None, "must only reveal one spk"); + (Some(revealed_spk), changeset) } - /// Gets the next unused script pubkey in the keychain. I.e., the script pubkey with the lowest - /// index that has not been used yet. + /// Gets the next unused script pubkey in the keychain. I.e. the script pubkey with the lowest + /// derivation index that has not been used (no associated transaction outputs). /// - /// This will derive and reveal a new script pubkey if no more unused script pubkeys exist. + /// Returns the unused script pubkey (alongside it's derivation index), and a [`ChangeSet`]. + /// The returned script pubkey will be `None` if the provided `keychain` has no associated + /// descriptor. /// - /// If the descriptor has no wildcard and already has a used script pubkey or if a descriptor - /// has used all scripts up to the derivation bounds, then the last derived script pubkey will be - /// returned. + /// This will derive and reveal a new script pubkey if no more unused script pubkeys exists + /// (refer to [`reveal_next_spk`](Self::reveal_next_spk)). /// - /// Returns None if the provided keychain doesn't exist. - pub fn next_unused_spk( - &mut self, - keychain: &K, - ) -> Option<((u32, &Script), super::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) + /// If the descriptor has no wildcard and already has a used script pubkey or if a descriptor + /// has used all scripts up to the derivation bounds, then the last derived script pubkey will + /// be returned. + pub fn next_unused_spk(&mut self, keychain: &K) -> (Option<(u32, ScriptBuf)>, ChangeSet) { + let next_unused_spk = self + .unused_keychain_spks(keychain) + .next() + .map(|(i, spk)| (i, spk.to_owned())); + if next_unused_spk.is_some() { + (next_unused_spk, Default::default()) } else { - Some(( - self.unused_keychain_spks(keychain) - .next() - .expect("we already know next exists"), - super::ChangeSet::default(), - )) + self.reveal_next_spk(keychain) } } @@ -908,9 +977,9 @@ impl KeychainTxOutIndex { bound: impl RangeBounds, ) -> impl RangeBounds<(DescriptorId, u32)> { let get_desc_id = |keychain| { - self.keychains_to_descriptors + self.keychains_to_descriptor_ids .get(keychain) - .map(|(desc_id, _)| *desc_id) + .copied() .unwrap_or_else(|| DescriptorId::from_byte_array([0; 32])) }; let start = match bound.start_bound() { @@ -936,7 +1005,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 +1020,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, diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 328163379..b266a5472 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -150,22 +150,19 @@ fn test_list_owned_txouts() { { // we need to scope here to take immutanble reference of the graph - for _ in 0..10 { - let ((_, script), _) = graph - .index - .reveal_next_spk(&"keychain_1".to_string()) - .unwrap(); - // TODO Assert indexes - trusted_spks.push(script.to_owned()); + for exp_i in 0_u32..10 { + let (next_spk, _) = graph.index.reveal_next_spk(&"keychain_1".to_string()); + let (spk_i, spk) = next_spk.expect("must exist"); + assert_eq!(spk_i, exp_i); + trusted_spks.push(spk); } } { - for _ in 0..10 { - let ((_, script), _) = graph - .index - .reveal_next_spk(&"keychain_2".to_string()) - .unwrap(); - untrusted_spks.push(script.to_owned()); + for exp_i in 0_u32..10 { + let (next_spk, _) = graph.index.reveal_next_spk(&"keychain_2".to_string()); + let (spk_i, spk) = next_spk.expect("must exist"); + assert_eq!(spk_i, exp_i); + untrusted_spks.push(spk); } } diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index 55af741c6..3b549413f 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -186,9 +186,9 @@ fn test_lookahead() { // - scripts cached in spk_txout_index should increase correctly // - stored scripts of external keychain should be of expected counts for index in (0..20).skip_while(|i| i % 2 == 1) { - let (revealed_spks, revealed_changeset) = txout_index - .reveal_to_target(&TestKeychain::External, index) - .unwrap(); + let (revealed_spks, revealed_changeset) = + txout_index.reveal_to_target(&TestKeychain::External, index); + let revealed_spks = revealed_spks.expect("must exist"); assert_eq!( revealed_spks.collect::>(), vec![(index, spk_at_index(&external_descriptor, index))], @@ -237,9 +237,9 @@ fn test_lookahead() { // - derivation index is set ahead of current derivation index + lookahead // expect: // - scripts cached in spk_txout_index should increase correctly, a.k.a. no scripts are skipped - let (revealed_spks, revealed_changeset) = txout_index - .reveal_to_target(&TestKeychain::Internal, 24) - .unwrap(); + let (revealed_spks, revealed_changeset) = + txout_index.reveal_to_target(&TestKeychain::Internal, 24); + let revealed_spks = revealed_spks.expect("must exist"); assert_eq!( revealed_spks.collect::>(), (0..=24) @@ -403,11 +403,11 @@ fn test_wildcard_derivations() { // - derive_new() == ((0, ), keychain::ChangeSet) // - 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())); + let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External); + assert_eq!(spk, Some((0_u32, external_spk_0.clone()))); assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 0)].into()); - let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap(); - assert_eq!(spk, (0_u32, external_spk_0.as_script())); + let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External); + assert_eq!(spk, Some((0_u32, external_spk_0))); assert_eq!(&changeset.last_revealed, &[].into()); // - derived till 25 @@ -426,13 +426,13 @@ fn test_wildcard_derivations() { assert_eq!(txout_index.next_index(&TestKeychain::External).unwrap(), (26, true)); - let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External).unwrap(); - assert_eq!(spk, (26, external_spk_26.as_script())); + let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External); + assert_eq!(spk, Some((26, external_spk_26))); assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 26)].into()); - let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap(); - assert_eq!(spk, (16, external_spk_16.as_script())); + let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External); + assert_eq!(spk, Some((16, external_spk_16))); assert_eq!(&changeset.last_revealed, &[].into()); // - Use all the derived till 26. @@ -441,8 +441,8 @@ fn test_wildcard_derivations() { txout_index.mark_used(TestKeychain::External, index); }); - let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External).unwrap(); - assert_eq!(spk, (27, external_spk_27.as_script())); + let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External); + assert_eq!(spk, Some((27, external_spk_27))); assert_eq!(&changeset.last_revealed, &[(external_descriptor.descriptor_id(), 27)].into()); } @@ -470,19 +470,15 @@ fn test_non_wildcard_derivations() { txout_index.next_index(&TestKeychain::External).unwrap(), (0, true) ); - let (spk, changeset) = txout_index - .reveal_next_spk(&TestKeychain::External) - .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External); + assert_eq!(spk, Some((0, external_spk.clone()))); assert_eq!( &changeset.last_revealed, &[(no_wildcard_descriptor.descriptor_id(), 0)].into() ); - let (spk, changeset) = txout_index - .next_unused_spk(&TestKeychain::External) - .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External); + assert_eq!(spk, Some((0, external_spk.clone()))); assert_eq!(&changeset.last_revealed, &[].into()); // given: @@ -497,21 +493,16 @@ fn test_non_wildcard_derivations() { ); txout_index.mark_used(TestKeychain::External, 0); - let (spk, changeset) = txout_index - .reveal_next_spk(&TestKeychain::External) - .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + let (spk, changeset) = txout_index.reveal_next_spk(&TestKeychain::External); + assert_eq!(spk, Some((0, external_spk.clone()))); assert_eq!(&changeset.last_revealed, &[].into()); - let (spk, changeset) = txout_index - .next_unused_spk(&TestKeychain::External) - .unwrap(); - assert_eq!(spk, (0, external_spk.as_script())); + let (spk, changeset) = txout_index.next_unused_spk(&TestKeychain::External); + assert_eq!(spk, Some((0, external_spk))); assert_eq!(&changeset.last_revealed, &[].into()); - let (revealed_spks, revealed_changeset) = txout_index - .reveal_to_target(&TestKeychain::External, 200) - .unwrap(); - assert_eq!(revealed_spks.count(), 0); + let (revealed_spks, revealed_changeset) = + txout_index.reveal_to_target(&TestKeychain::External, 200); + assert_eq!(revealed_spks.map(Iterator::count), Some(0)); assert!(revealed_changeset.is_empty()); // we check that spks_of_keychain returns a SpkIterator with just one element @@ -754,18 +745,16 @@ fn test_only_highest_ord_keychain_is_returned() { let _ = indexer.insert_descriptor(TestKeychain::External, desc); // reveal_next_spk will work with either keychain - let spk0: ScriptBuf = indexer - .reveal_next_spk(&TestKeychain::External) - .unwrap() - .0 - .1 - .into(); - let spk1: ScriptBuf = indexer - .reveal_next_spk(&TestKeychain::Internal) - .unwrap() - .0 - .1 - .into(); + let spk0 = { + let (spk, _) = indexer.reveal_next_spk(&TestKeychain::External); + let (_, spk) = spk.expect("must exist"); + spk + }; + let spk1 = { + let (spk, _) = indexer.reveal_next_spk(&TestKeychain::Internal); + let (_, spk) = spk.expect("must exist"); + spk + }; // index_of_spk will always return External assert_eq!( diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 61ec5893b..8145034af 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -171,6 +171,15 @@ impl Append for ChangeSet { } } +impl From> for ChangeSet { + fn from(keychain_changeset: keychain::ChangeSet) -> Self { + Self { + indexed_tx_graph: keychain_changeset.into(), + ..Default::default() + } + } +} + impl From for ChangeSet { fn from(chain: local_chain::ChangeSet) -> Self { Self { @@ -813,18 +822,14 @@ impl Wallet { /// If writing to persistent storage fails. pub fn reveal_next_address(&mut self, keychain: KeychainKind) -> anyhow::Result { let keychain = self.map_keychain(keychain); - let ((index, spk), index_changeset) = self - .indexed_graph - .index - .reveal_next_spk(&keychain) - .expect("Must exist (we called map_keychain)"); - + let (next_spk, index_changeset) = self.indexed_graph.index.reveal_next_spk(&keychain); + let (index, spk) = next_spk.expect("Must exist (we called map_keychain)"); self.persist .stage_and_commit(indexed_tx_graph::ChangeSet::from(index_changeset).into())?; Ok(AddressInfo { index, - address: Address::from_script(spk, self.network).expect("must have address form"), + address: Address::from_script(&spk, self.network).expect("must have address form"), keychain, }) } @@ -845,11 +850,9 @@ impl Wallet { index: u32, ) -> anyhow::Result + '_> { let keychain = self.map_keychain(keychain); - let (spk_iter, index_changeset) = self - .indexed_graph - .index - .reveal_to_target(&keychain, index) - .expect("must exist (we called map_keychain)"); + let (spk_iter, index_changeset) = + self.indexed_graph.index.reveal_to_target(&keychain, index); + let spk_iter = spk_iter.expect("Must exist (we called map_keychain)"); self.persist .stage_and_commit(indexed_tx_graph::ChangeSet::from(index_changeset).into())?; @@ -872,18 +875,15 @@ impl Wallet { /// If writing to persistent storage fails. pub fn next_unused_address(&mut self, keychain: KeychainKind) -> anyhow::Result { let keychain = self.map_keychain(keychain); - let ((index, spk), index_changeset) = self - .indexed_graph - .index - .next_unused_spk(&keychain) - .expect("must exist (we called map_keychain)"); + let (next_spk, index_changeset) = self.indexed_graph.index.next_unused_spk(&keychain); + let (index, spk) = next_spk.expect("Must exist (we called map_keychain)"); self.persist .stage_and_commit(indexed_tx_graph::ChangeSet::from(index_changeset).into())?; Ok(AddressInfo { index, - address: Address::from_script(spk, self.network).expect("must have address form"), + address: Address::from_script(&spk, self.network).expect("must have address form"), keychain, }) } @@ -1038,7 +1038,7 @@ impl Wallet { /// [`commit`]: Self::commit pub fn insert_txout(&mut self, outpoint: OutPoint, txout: TxOut) { let additions = self.indexed_graph.insert_txout(outpoint, txout); - self.persist.stage(ChangeSet::from(additions)); + self.persist.stage(additions.into()); } /// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction. @@ -1607,17 +1607,11 @@ impl Wallet { Some(ref drain_recipient) => drain_recipient.clone(), None => { let change_keychain = self.map_keychain(KeychainKind::Internal); - let ((index, spk), index_changeset) = self - .indexed_graph - .index - .next_unused_spk(&change_keychain) - .expect("Keychain exists (we called map_keychain)"); - let spk = spk.into(); + let (next_spk, index_changeset) = + self.indexed_graph.index.next_unused_spk(&change_keychain); + let (index, spk) = next_spk.expect("Keychain exists (we called map_keychain)"); self.indexed_graph.index.mark_used(change_keychain, index); - self.persist - .stage(ChangeSet::from(indexed_tx_graph::ChangeSet::from( - index_changeset, - ))); + self.persist.stage(index_changeset.into()); self.persist.commit().map_err(CreateTxError::Persist)?; spk } @@ -2432,21 +2426,19 @@ impl Wallet { /// [`commit`]: Self::commit pub fn apply_update(&mut self, update: impl Into) -> Result<(), CannotConnectError> { let update = update.into(); - let mut changeset = match update.chain { - Some(chain_update) => ChangeSet::from(self.chain.apply_update(chain_update)?), - None => ChangeSet::default(), - }; + let mut changeset = ChangeSet::default(); - let (_, index_changeset) = self - .indexed_graph - .index - .reveal_to_target_multi(&update.last_active_indices); - changeset.append(ChangeSet::from(indexed_tx_graph::ChangeSet::from( - index_changeset, - ))); - changeset.append(ChangeSet::from( - self.indexed_graph.apply_update(update.graph), - )); + if let Some(chain_update) = update.chain { + changeset.append(self.chain.apply_update(chain_update)?.into()); + } + changeset.append({ + let (_, index_changeset) = self + .indexed_graph + .index + .reveal_to_target_multi(&update.last_active_indices); + index_changeset.into() + }); + changeset.append(self.indexed_graph.apply_update(update.graph).into()); self.persist.stage(changeset); Ok(()) } @@ -2552,7 +2544,7 @@ impl Wallet { let indexed_graph_changeset = self .indexed_graph .batch_insert_relevant_unconfirmed(unconfirmed_txs); - self.persist.stage(ChangeSet::from(indexed_graph_changeset)); + self.persist.stage(indexed_graph_changeset.into()); } } diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 319c23805..ca30ac5f8 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -259,15 +259,10 @@ where Keychain::External }; - let ((change_index, change_script), change_changeset) = graph - .index - .next_unused_spk(&internal_keychain) - .expect("Must exist"); + let (change_next_spk, change_changeset) = graph.index.next_unused_spk(&internal_keychain); + let (change_index, change_script) = change_next_spk.expect("must have next spk"); 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 @@ -474,15 +469,15 @@ where _ => unreachable!("only these two variants exist in match arm"), }; - let ((spk_i, spk), index_changeset) = - spk_chooser(index, &Keychain::External).expect("Must exist"); + let (next_spk, index_changeset) = spk_chooser(index, &Keychain::External); + let (spk_i, spk) = next_spk.expect("keychain must exist"); let db = &mut *db.lock().unwrap(); db.stage_and_commit(C::from(( local_chain::ChangeSet::default(), indexed_tx_graph::ChangeSet::from(index_changeset), )))?; let addr = - Address::from_script(spk, network).context("failed to derive address")?; + Address::from_script(&spk, network).context("failed to derive address")?; println!("[address @ {}] {}", spk_i, addr); Ok(()) }