feat(discovery-core): add batch iviews methods and outgoing channel support - #457
Conversation
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: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @m-kus).
crates/discovery-core/src/storage_backend.rs line 43 at r1 (raw file):
/// /// The returned `Vec` must have the same length as `slots`. async fn read_slots(&self, slots: Vec<Felt>) -> Result<Vec<Felt>, StorageError>;
Consider this
Suggestion:
}
/// Low-level storage access for reading raw storage slots.
#[async_trait]
pub trait RawStorageAccess: Send + Sync {
/// Reads a single storage slot.
async fn read_slot(&self, slot: Felt) -> Result<Felt, StorageError>;
/// Reads multiple storage slots.
async fn read_slots<const N: usize>(&self, slots: [Felt; N]) -> Result<[Felt; N], StorageError>;
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware made 2 comments.
Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @m-kus).
crates/discovery-core/src/privacy_pool/views.rs line 86 at r1 (raw file):
nullifiers: &[Felt], ) -> Result<(Vec<Felt>, Vec<bool>), StorageError>; }
WDYT about setting a default impl in this trait for all non-batch funcs to call their batch variant with count=1?
Code quote:
/// Batch-reads channel info for `count` consecutive channels starting at `start_index`.
///
/// Returns a `Vec<EncChannelInfo>` of length `count`, fetched in a single `read_slots` call.
async fn get_channel_info_batch(
&self,
recipient_addr: Felt,
start_index: u64,
count: usize,
) -> Result<Vec<EncChannelInfo>, StorageError>;
/// Batch-reads packed note values for the given note IDs.
///
/// Returns a `Vec<Felt>` matching the input length. Zero = note doesn't exist.
async fn get_notes_batch(&self, note_ids: &[Felt]) -> Result<Vec<Felt>, StorageError>;
/// Batch-reads public keys for the given addresses.
///
/// Returns a `Vec<Felt>` matching the input length. Zero = unregistered.
async fn get_public_keys_batch(&self, addrs: &[Felt]) -> Result<Vec<Felt>, StorageError>;
/// Batch-reads packed note amounts and nullifier existence.
///
/// Returns `(packed_amounts, nullifier_exists)`.
/// Both vectors match the lengths of their respective inputs.
async fn get_note_and_nullifier_batch(
&self,
note_ids: &[Felt],
nullifiers: &[Felt],
) -> Result<(Vec<Felt>, Vec<bool>), StorageError>;
}crates/discovery-core/src/privacy_pool/views.rs line 169 at r1 (raw file):
let slots = storage_slots::outgoing_channels(outgoing_channel_id); let values = self .read_slots(vec![slots.salt, slots.enc_recipient_addr])
A bit weird that you unpack the slots struct here. That's another place to fix if the format changes.
Consider returning an array, or doing this unpacking through some into/to_vec method of this struct.
Code quote:
let slots = storage_slots::outgoing_channels(outgoing_channel_id);
let values = self
.read_slots(vec![slots.salt, slots.enc_recipient_addr])2fc4a74 to
b444378
Compare
2a6b820 to
52e1fa9
Compare
52e1fa9 to
64df6c3
Compare
b444378 to
ec15ebd
Compare
m-kus
left a comment
There was a problem hiding this comment.
@m-kus made 2 comments and resolved 1 discussion.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware).
crates/discovery-core/src/storage_backend.rs line 43 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Consider this
batch size is dynamic, we don't know it in advance
crates/discovery-core/src/privacy_pool/views.rs line 86 at r1 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
WDYT about setting a default impl in this trait for all non-batch funcs to call their batch variant with count=1?
Done.
ec15ebd to
f2a60f2
Compare
64df6c3 to
4ddeb8f
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 2 files and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @m-kus).
crates/discovery-core/src/storage_backend.rs line 43 at r1 (raw file):
Previously, m-kus (Michael Zaikin) wrote…
batch size is dynamic, we don't know it in advance
:(
crates/discovery-core/src/privacy_pool/views.rs line 178 at r2 (raw file):
#[tracing::instrument(name = "get_note", level = "debug", skip(self))] async fn get_note(&self, note_id: Felt) -> Result<Felt, StorageError> {
Delete the non-batch impls
Code quote:
async fn get_note(&self, note_id: Felt) -> Result<Felt, StorageError> {
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware made 3 comments.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @m-kus).
crates/discovery-core/src/privacy_pool/views.rs line 178 at r2 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Delete the non-batch impls
.
crates/discovery-core/src/privacy_pool/views.rs line 290 at r2 (raw file):
let values = self.read_slots(slots).await?; check_slots_len(&values, nullifiers.len())?; Ok(values.iter().map(|v| *v != Felt::ZERO).collect())
read_slots guarantees the output length. No need to check again if you don't need to load into a struct
Suggestion:
async fn get_notes_batch(&self, note_ids: &[Felt]) -> Result<Vec<Felt>, StorageError> {
let slots: Vec<_> = note_ids
.iter()
.map(|&nid| storage_slots::notes(nid))
.collect();
let values = self.read_slots(slots).await?;
Ok(values)
}
#[tracing::instrument(
name = "get_public_keys_batch",
level = "debug",
skip(self, addrs),
fields(count = addrs.len())
)]
async fn get_public_keys_batch(&self, addrs: &[Felt]) -> Result<Vec<Felt>, StorageError> {
let slots: Vec<_> = addrs
.iter()
.map(|&addr| storage_slots::public_key(addr))
.collect();
let values = self.read_slots(slots).await?;
Ok(values)
}
#[tracing::instrument(
name = "nullifier_exists_batch",
level = "debug",
skip(self, nullifiers),
fields(count = nullifiers.len())
)]
async fn nullifier_exists_batch(&self, nullifiers: &[Felt]) -> Result<Vec<bool>, StorageError> {
let slots: Vec<_> = nullifiers
.iter()
.map(|&nul| storage_slots::nullifiers(nul))
.collect();
let values = self.read_slots(slots).await?;
Ok(values.iter().map(|v| *v != Felt::ZERO).collect())
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 1 file.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @m-kus).
f2a60f2 to
269dbcc
Compare
4ddeb8f to
3a3512a
Compare
m-kus
left a comment
There was a problem hiding this comment.
@m-kus resolved 2 discussions.
Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @Yoni-Starkware).
3a3512a to
3e9392f
Compare

TL;DR
Added support for batch operations and outgoing channel information to the privacy pool implementation.
What changed?
EncOutgoingChannelInfostruct to represent encrypted outgoing channel informationget_outgoing_channel_infoto retrieve outgoing channel dataget_channel_info_batch: Retrieves multiple channel infos in one callget_notes_batch: Retrieves multiple note values in one callget_public_keys_batch: Retrieves multiple public keys in one callget_note_and_nullifier_batch: Retrieves both note amounts and nullifier existence in one callSlotCountMismatcherror type to handle cases where the wrong number of slots are returnedThis change is