feat(discovery-core): add paginated last-note-index search with bisection - #471
Conversation
faed129 to
40a6e4f
Compare
d26d18c to
36ea022
Compare
36ea022 to
c0935dc
Compare
40a6e4f to
30a7894
Compare
c0935dc to
c7d55fa
Compare
30a7894 to
7b2de5f
Compare
7b2de5f to
92649eb
Compare
c7d55fa to
608f58b
Compare
92649eb to
b691a09
Compare
6ec7f8a to
ba192db
Compare
23bdf84 to
c487ff1
Compare
ba192db to
b31abb3
Compare
c487ff1 to
cd0122e
Compare
b31abb3 to
76537a9
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 1 file and all commit messages, and made 6 comments.
Reviewable status: 1 of 2 files reviewed, 6 unresolved discussions (waiting on m-kus).
crates/discovery-core/src/discovery/last_note_index.rs line 48 at r1 (raw file):
/// /// Returns `(last_index, has_more)`. When `has_more` is `false`, the search is complete. pub async fn find_last_note_index_paginated<S: IViews>(
Can you point me to the PR that uses this function?
(I don't see where you propagate the cache)
crates/discovery-core/src/discovery/last_note_index.rs line 57 at r1 (raw file):
// Phase 1: Ascending (find bounds) if cursor.max_note_index.is_none() { let start = cursor.last_note_index.map_or(0, |lo| lo + 1);
Suggestion:
let start = cursor.start_index();crates/discovery-core/src/discovery/last_note_index.rs line 76 at r1 (raw file):
if cursor.last_note_index.is_none() { return Ok((None, false)); }
probe_complete already considers result.last_found_index.is_some()
Suggestion:
// Nothing to bisect.
if cursor.max_note_index.is_none() || cursor.last_note_index.is_none() {
let has_more = !result.probe_complete
return Ok((cursor.last_note_index, has_more));
}crates/discovery-core/src/discovery/last_note_index.rs line 80 at r1 (raw file):
// Phase 2: Bisection (narrow down to exact boundary, sequential) let probe = |idx: u64| {
Please document the return value, also in the bisect_boundary docstring
Code quote:
// Phase 2: Bisection (narrow down to exact boundary, sequential)
let probe = |idx: u64| {crates/discovery-core/src/discovery/last_note_index.rs line 113 at r1 (raw file):
(Some(lo), Some(hi)) => (lo, hi), _ => return Ok(true), // Nothing to bisect };
The _ should be unreachable, please enforce that
Code quote:
let (mut lo, mut hi) = match (cursor.last_note_index, cursor.max_note_index) {
(Some(lo), Some(hi)) => (lo, hi),
_ => return Ok(true), // Nothing to bisect
};crates/discovery-core/src/discovery/last_note_index.rs line 174 at r1 (raw file):
.map(|exp| (1u64 << exp) - 1) .take_while(|&off| off <= range && off <= offset_cap), )
Please revert to the previous terminology (upper_bound -> max_index, off -> offset, etc)
Code quote:
let offset_cap = max_probe_offset.unwrap_or(u64::MAX);
let upper_bound = match prior_max_index {
None => start_index.saturating_add(offset_cap),
Some(m) => m.saturating_mul(2).max(start_index),
};
let range = upper_bound.saturating_sub(start_index);
// Offsets: [0, 1, 3, 7, 15, ..., 2^k - 1] where 2^k - 1 <= min(range, offset_cap).
let offsets: Vec<u64> = std::iter::once(0)
.chain(
(1..64)
.map(|exp| (1u64 << exp) - 1)
.take_while(|&off| off <= range && off <= offset_cap),
)76537a9 to
ca63ab0
Compare
ca63ab0 to
32b8a83
Compare
m-kus
left a comment
There was a problem hiding this comment.
@m-kus made 1 comment and resolved 5 discussions.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on Yoni-Starkware).
crates/discovery-core/src/discovery/last_note_index.rs line 48 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Can you point me to the PR that uses this function?
(I don't see where you propagate the cache)
probe cache is used in notes discovery; find_last_note_index_paginated will be used in outgoing state sync (upcoming PR #483)
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on m-kus).
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on m-kus).
32b8a83 to
f46ae8c
Compare
3751228 to
5f75584
Compare
f46ae8c to
7aeec0c
Compare

Improved Note Index Discovery with Paginated Last-Note-Index Search
TL;DR
Refactored note index discovery to support paginated last-note-index search for outgoing channels, with improved exponential probing and bisection algorithms.
What changed?
exponential_ascendtoexponential_probeand made it more flexible with configurable max probe offsetfind_last_note_index_paginatedfunction that combines exponential probing and binary search to efficiently find the last note indexbisect_boundaryhelper for binary search between known boundsMAX_PROBE_OFFSETconfigurable viaDEFAULT_MAX_PROBE_OFFSETconstantThis change is