refactor(indexer): reorganize discovery-core, prepare for state sync - #407
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e8f1cb5 to
b7bd5d0
Compare
b7bd5d0 to
d828a86
Compare
4efbe71 to
288a85b
Compare
d828a86 to
dd9ee93
Compare
0591199 to
9aafdbc
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware partially reviewed 9 files and all commit messages, and made 1 comment.
Reviewable status: 9 of 20 files reviewed, 1 unresolved discussion (waiting on @m-kus).
crates/discovery-core/src/discovery/subchannels.rs line 31 at r4 (raw file):
/// Index of the last discovered subchannel, or `None` if no /// subchannels were discovered. pub last_index: Option<u64>,
Since it's a function of subchannels, WDYT about removing this field? on all responses.
(you can add a method)
Code quote:
/// Index of the last discovered subchannel, or `None` if no
/// subchannels were discovered.
pub last_index: Option<u64>,
m-kus
left a comment
There was a problem hiding this comment.
@m-kus made 1 comment.
Reviewable status: 9 of 20 files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware).
crates/discovery-core/src/discovery/subchannels.rs line 31 at r4 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Since it's a function of
subchannels, WDYT about removing this field? on all responses.
(you can add a method)
last index is start index + number of all newly discovered subchannels. If we complete discovery in a single request then we could derive last index but in general case we cannot.
9aafdbc to
9f5c504
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware partially reviewed 11 files and made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @m-kus).
crates/discovery-core/src/discovery/subchannels.rs line 31 at r4 (raw file):
Previously, m-kus (Michael Zaikin) wrote…
last index is start index + number of all newly discovered subchannels. If we complete discovery in a single request then we could derive last index but in general case we cannot.
I don't understand - I see that last_index = subchannels.last().map(|s| s.index);, and that's the only flow, whether you ran out of budget or not.
Anyway, non-blocking for the stack, but seems redundant
crates/discovery-core/src/privacy_pool/hashes.rs line 162 at r4 (raw file):
#[test] fn test_compute_enc_amount_hash() { use crate::privacy_pool::types::felt_low_u128;
Non-blocking (don't fix in this stack): our convention is to put all imports in the top of the file
Code quote:
fn test_compute_enc_amount_hash() {
use crate::privacy_pool::types::felt_low_u128;crates/discovery-core/src/privacy_pool/decryption.rs line 9 at r4 (raw file):
use thiserror::Error; use super::hashes::{
Non-blocking (don't fix in this stack): our convention is to avoid super
Code quote:
use super::hashes::{
m-kus
left a comment
There was a problem hiding this comment.
@m-kus made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware).
crates/discovery-core/src/discovery/subchannels.rs line 31 at r4 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
I don't understand - I see that
last_index = subchannels.last().map(|s| s.index);, and that's the only flow, whether you ran out of budget or not.Anyway, non-blocking for the stack, but seems redundant
Ah right! You are correct, it's currently redundant, but semantically it's better to separate because generally subchannel discovery is not obliged to return everything it found, e.g. sdk now supports filtering by token (which discovery service does not yet implement)
crates/discovery-core/src/privacy_pool/decryption.rs line 9 at r4 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Non-blocking (don't fix in this stack): our convention is to avoid
super
Noted
crates/discovery-core/src/privacy_pool/hashes.rs line 162 at r4 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Non-blocking (don't fix in this stack): our convention is to put all imports in the top of the file
Noted, I'll do mass refactoring wrt imports afterwards
9f5c504 to
07a46a6
Compare

TL;DR
Restructured the discovery-core crate to improve organization and reduce dependencies.
What changed?
anyhowdependency from discovery-coreprivacy_poolnamespaceDiscoveryResulttoChannelDiscoveryResultfor claritytotal_n_channels/subchannels/notestolast_indexto better represent their purposeio_budget.rstodiscovery/mod.rsstorage_backendmoduleHow to test?
Why make this change?
This refactoring improves code organization by clearly separating privacy pool-specific code from the general discovery framework. It also makes the API more explicit by separating the channel count fetching from the discovery process, which allows for better error handling and budget management. Removing the
anyhowdependency reduces the dependency footprint of the crate.This change is