From 21d88758262bfbc3d9d2109aac9883f23ab651c4 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Mon, 8 Jan 2024 14:46:25 +0100 Subject: [PATCH 1/3] ref(chain): Refactor next_store_index Rename `v` to `index` for better clarity, and add a comment explaining the `range` --- crates/chain/src/keychain/txout_index.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 3287d4f2..03665f21 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -206,9 +206,11 @@ impl KeychainTxOutIndex { fn next_store_index(&self, keychain: &K) -> u32 { self.inner() .all_spks() + // This range is filtering out the spks with a keychain different than + // `keychain`. We don't use filter here as range is more optimized. .range((keychain.clone(), u32::MIN)..(keychain.clone(), u32::MAX)) .last() - .map_or(0, |((_, v), _)| *v + 1) + .map_or(0, |((_, index), _)| *index + 1) } /// Generates script pubkey iterators for every `keychain`. The iterators iterate over all From 887e112e8f4ee5300bf09a0f3590861bff7fd053 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Mon, 8 Jan 2024 14:47:28 +0100 Subject: [PATCH 2/3] ref(chain): Refactor reveal_to_target Simplify the `reveal_to_target` algorithm by exiting prematurely if the `target_index` is already revealed. Since the `reveal_to_index` variable was different from `Some(target_index)` only if the target was already revealed, we can getrid of the variable altogether. --- crates/chain/src/keychain/txout_index.rs | 54 ++++++++++-------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs index 03665f21..5c9eb084 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -364,51 +364,41 @@ impl KeychainTxOutIndex { let has_wildcard = descriptor.has_wildcard(); let target_index = if has_wildcard { target_index } else { 0 }; - let next_reveal_index = self.last_revealed.get(keychain).map_or(0, |v| *v + 1); + let next_reveal_index = self + .last_revealed + .get(keychain) + .map_or(0, |index| *index + 1); debug_assert!(next_reveal_index + self.lookahead >= self.next_store_index(keychain)); - // if we need to reveal new indices, the latest revealed index goes here - let mut reveal_to_index = None; - - // if the target is not yet revealed, but is already stored (due to lookahead), we need to - // set the `reveal_to_index` as target here (as the `for` loop below only updates - // `reveal_to_index` for indexes that are NOT stored) - if next_reveal_index <= target_index && target_index < next_reveal_index + self.lookahead { - reveal_to_index = Some(target_index); + // If the target_index is already revealed, we are done + if next_reveal_index > target_index { + return ( + SpkIterator::new_with_range( + descriptor.clone(), + next_reveal_index..next_reveal_index, + ), + super::ChangeSet::default(), + ); } - // we range over indexes that are not stored + // 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, range) { let _inserted = self .inner .insert_spk((keychain.clone(), new_index), new_spk); debug_assert!(_inserted, "must not have existing spk",); - - // everything after `target_index` is stored for lookahead only - if new_index <= target_index { - reveal_to_index = Some(new_index); - } } - match reveal_to_index { - Some(index) => { - let _old_index = self.last_revealed.insert(keychain.clone(), index); - debug_assert!(_old_index < Some(index)); - ( - SpkIterator::new_with_range(descriptor.clone(), next_reveal_index..index + 1), - super::ChangeSet(core::iter::once((keychain.clone(), index)).collect()), - ) - } - None => ( - SpkIterator::new_with_range( - descriptor.clone(), - next_reveal_index..next_reveal_index, - ), - super::ChangeSet::default(), - ), - } + let _old_index = self.last_revealed.insert(keychain.clone(), target_index); + debug_assert!(_old_index < Some(target_index)); + ( + SpkIterator::new_with_range(descriptor.clone(), next_reveal_index..target_index + 1), + super::ChangeSet(core::iter::once((keychain.clone(), target_index)).collect()), + ) } /// Attempts to reveal the next script pubkey for `keychain`. From 761189ab2b679fbfee45b33435d66ed98a135e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 9 Jan 2024 09:44:06 +0800 Subject: [PATCH 3/3] feat(chain): debug_assert non-wildcard desc. to only cache index 0 --- 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 5c9eb084..a8955b8a 100644 --- a/crates/chain/src/keychain/txout_index.rs +++ b/crates/chain/src/keychain/txout_index.rs @@ -390,7 +390,11 @@ impl KeychainTxOutIndex { let _inserted = self .inner .insert_spk((keychain.clone(), new_index), new_spk); - debug_assert!(_inserted, "must not have existing 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(keychain.clone(), target_index);