Skip to content

Commit c3544c9

Browse files
committed
Merge #1349: Fix KeychainTxOutIndex::lookahead_to_target
b290b29 test(chain): change test case comments to docstring (志宇) c151d8f fix(chain): `KeychainTxOutIndex::lookahead_to_target` (志宇) Pull request description: ### Description This method was not used (so it was untested) and it was not working. This fixes it. The old implementation used `.next_store_index` which returned the keychain's last index stored in `.inner` (which include lookahead spks). This is WRONG because `.replenish_lookahead` needs the difference from last revealed. ### Changelog notice Fix `KeychainTxOutIndex::lookahead_to_target` ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### Bugfixes: * [x] I've added tests to reproduce the issue which are now passing ACKs for top commit: notmandatory: ACK b290b29 Tree-SHA512: af50c6af18b6b57494cfa37f89b0236674fa331091d791e858f67b7d0b3a1e4e11e7422029bd6a2dc1c795914cdf6d592a14b42a62ca7c7c475ba6ed37182539
2 parents d77a7f2 + b290b29 commit c3544c9

File tree

2 files changed

+109
-4
lines changed

2 files changed

+109
-4
lines changed

crates/chain/src/keychain/txout_index.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,17 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
326326
self.lookahead
327327
}
328328

329-
/// Store lookahead scripts until `target_index`.
329+
/// Store lookahead scripts until `target_index` (inclusive).
330330
///
331-
/// This does not change the `lookahead` setting.
331+
/// This does not change the global `lookahead` setting.
332332
pub fn lookahead_to_target(&mut self, keychain: &K, target_index: u32) {
333-
let next_index = self.next_store_index(keychain);
334-
if let Some(temp_lookahead) = target_index.checked_sub(next_index).filter(|&v| v > 0) {
333+
let (next_index, _) = self.next_index(keychain);
334+
335+
let temp_lookahead = (target_index + 1)
336+
.checked_sub(next_index)
337+
.filter(|&index| index > 0);
338+
339+
if let Some(temp_lookahead) = temp_lookahead {
335340
self.replenish_lookahead(keychain, temp_lookahead);
336341
}
337342
}

crates/chain/tests/test_keychain_txout_index.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,3 +386,103 @@ fn test_non_wildcard_derivations() {
386386
1,
387387
);
388388
}
389+
390+
/// Check that calling `lookahead_to_target` stores the expected spks.
391+
#[test]
392+
fn lookahead_to_target() {
393+
#[derive(Default)]
394+
struct TestCase {
395+
/// Global lookahead value.
396+
lookahead: u32,
397+
/// Last revealed index for external keychain.
398+
external_last_revealed: Option<u32>,
399+
/// Last revealed index for internal keychain.
400+
internal_last_revealed: Option<u32>,
401+
/// Call `lookahead_to_target(External, u32)`.
402+
external_target: Option<u32>,
403+
/// Call `lookahead_to_target(Internal, u32)`.
404+
internal_target: Option<u32>,
405+
}
406+
407+
let test_cases = &[
408+
TestCase {
409+
lookahead: 0,
410+
external_target: Some(100),
411+
..Default::default()
412+
},
413+
TestCase {
414+
lookahead: 10,
415+
internal_target: Some(99),
416+
..Default::default()
417+
},
418+
TestCase {
419+
lookahead: 100,
420+
internal_target: Some(9),
421+
external_target: Some(10),
422+
..Default::default()
423+
},
424+
TestCase {
425+
lookahead: 12,
426+
external_last_revealed: Some(2),
427+
internal_last_revealed: Some(2),
428+
internal_target: Some(15),
429+
external_target: Some(13),
430+
},
431+
TestCase {
432+
lookahead: 13,
433+
external_last_revealed: Some(100),
434+
internal_last_revealed: Some(21),
435+
internal_target: Some(120),
436+
external_target: Some(130),
437+
},
438+
];
439+
440+
for t in test_cases {
441+
let (mut index, _, _) = init_txout_index(t.lookahead);
442+
443+
if let Some(last_revealed) = t.external_last_revealed {
444+
let _ = index.reveal_to_target(&TestKeychain::External, last_revealed);
445+
}
446+
if let Some(last_revealed) = t.internal_last_revealed {
447+
let _ = index.reveal_to_target(&TestKeychain::Internal, last_revealed);
448+
}
449+
450+
let keychain_test_cases = [
451+
(
452+
TestKeychain::External,
453+
t.external_last_revealed,
454+
t.external_target,
455+
),
456+
(
457+
TestKeychain::Internal,
458+
t.internal_last_revealed,
459+
t.internal_target,
460+
),
461+
];
462+
for (keychain, last_revealed, target) in keychain_test_cases {
463+
if let Some(target) = target {
464+
let original_last_stored_index = match last_revealed {
465+
Some(last_revealed) => Some(last_revealed + t.lookahead),
466+
None => t.lookahead.checked_sub(1),
467+
};
468+
let exp_last_stored_index = match original_last_stored_index {
469+
Some(original_last_stored_index) => {
470+
Ord::max(target, original_last_stored_index)
471+
}
472+
None => target,
473+
};
474+
index.lookahead_to_target(&keychain, target);
475+
let keys = index
476+
.inner()
477+
.all_spks()
478+
.range((keychain.clone(), 0)..=(keychain.clone(), u32::MAX))
479+
.map(|(k, _)| k.clone())
480+
.collect::<Vec<_>>();
481+
let exp_keys = core::iter::repeat(keychain)
482+
.zip(0_u32..=exp_last_stored_index)
483+
.collect::<Vec<_>>();
484+
assert_eq!(keys, exp_keys);
485+
}
486+
}
487+
}
488+
}

0 commit comments

Comments
 (0)