From 8d5e93f4361faee71741102b7d9a2110a32f2050 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Tue, 28 Jan 2025 15:02:26 -0300 Subject: [PATCH 01/14] feat: remove steps from state sync interface --- crates/rust-client/src/account.rs | 7 +- crates/rust-client/src/note/mod.rs | 109 ++++--- .../src/store/sqlite_store/note.rs | 10 +- .../src/store/sqlite_store/sync.rs | 30 +- crates/rust-client/src/sync/mod.rs | 112 ++++--- crates/rust-client/src/sync/state_sync.rs | 281 ++++++++++-------- crates/rust-client/src/transaction/mod.rs | 4 +- 7 files changed, 299 insertions(+), 254 deletions(-) diff --git a/crates/rust-client/src/account.rs b/crates/rust-client/src/account.rs index 60ceb6981..4618aaa81 100644 --- a/crates/rust-client/src/account.rs +++ b/crates/rust-client/src/account.rs @@ -256,7 +256,7 @@ impl Client { // ACCOUNT UPDATES // ================================================================================================ -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] /// Contains account changes to apply to the store. pub struct AccountUpdates { /// Updated public accounts. @@ -286,6 +286,11 @@ impl AccountUpdates { pub fn mismatched_private_accounts(&self) -> &[(AccountId, Digest)] { &self.mismatched_private_accounts } + + pub fn extend(&mut self, other: AccountUpdates) { + self.updated_public_accounts.extend(other.updated_public_accounts); + self.mismatched_private_accounts.extend(other.mismatched_private_accounts); + } } // TESTS diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 433db9570..d26dad6fa 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -57,6 +57,7 @@ //! and types in this module. use alloc::{collections::BTreeSet, string::ToString, vec::Vec}; +use std::collections::BTreeMap; use miden_lib::transaction::TransactionKernel; use miden_objects::{account::AccountId, crypto::rand::FeltRng}; @@ -244,89 +245,63 @@ pub async fn get_input_note_with_id_prefix( /// Contains note changes to apply to the store. #[derive(Clone, Debug, Default)] pub struct NoteUpdates { - /// A list of new input notes. - new_input_notes: Vec, - /// A list of new output notes. - new_output_notes: Vec, - /// A list of updated input note records corresponding to locally-tracked input notes. - updated_input_notes: Vec, - /// A list of updated output note records corresponding to locally-tracked output notes. - updated_output_notes: Vec, + /// A map of updated input note records corresponding to locally-tracked input notes. + updated_input_notes: BTreeMap, + /// A map of updated output note records corresponding to locally-tracked output notes. + updated_output_notes: BTreeMap, } impl NoteUpdates { /// Creates a [NoteUpdates]. pub fn new( - new_input_notes: Vec, - new_output_notes: Vec, updated_input_notes: Vec, updated_output_notes: Vec, ) -> Self { Self { - new_input_notes, - new_output_notes, - updated_input_notes, - updated_output_notes, + updated_input_notes: updated_input_notes + .into_iter() + .map(|note| (note.id(), note)) + .collect(), + updated_output_notes: updated_output_notes + .into_iter() + .map(|note| (note.id(), note)) + .collect(), } } - /// Combines two [NoteUpdates] into a single one. - pub fn extend(&mut self, other: Self) { - self.new_input_notes.extend(other.new_input_notes); - self.new_output_notes.extend(other.new_output_notes); - self.updated_input_notes.extend(other.updated_input_notes); - self.updated_output_notes.extend(other.updated_output_notes); - } - - /// Returns all new input note records, meant to be tracked by the client. - pub fn new_input_notes(&self) -> &[InputNoteRecord] { - &self.new_input_notes - } - - /// Returns all new output note records, meant to be tracked by the client. - pub fn new_output_notes(&self) -> &[OutputNoteRecord] { - &self.new_output_notes - } - /// Returns all updated input note records. That is, any input notes that are locally tracked /// and have been updated. - pub fn updated_input_notes(&self) -> &[InputNoteRecord] { - &self.updated_input_notes + pub fn updated_input_notes(&self) -> impl Iterator { + self.updated_input_notes.values() } /// Returns all updated output note records. That is, any output notes that are locally tracked /// and have been updated. - pub fn updated_output_notes(&self) -> &[OutputNoteRecord] { - &self.updated_output_notes + pub fn updated_output_notes(&self) -> impl Iterator { + self.updated_output_notes.values() } /// Returns whether no new note-related information has been retrieved. pub fn is_empty(&self) -> bool { - self.updated_input_notes.is_empty() - && self.updated_output_notes.is_empty() - && self.new_input_notes.is_empty() - && self.new_output_notes.is_empty() + self.updated_input_notes.is_empty() && self.updated_output_notes.is_empty() } /// Returns any note that has been committed into the chain in this update (either new or /// already locally tracked) pub fn committed_input_notes(&self) -> impl Iterator { - self.updated_input_notes - .iter() - .chain(self.new_input_notes.iter()) - .filter(|note| note.is_committed()) + self.updated_input_notes.values().filter(|note| note.is_committed()) } /// Returns the IDs of all notes that have been committed (previously locally tracked). pub fn committed_note_ids(&self) -> BTreeSet { let committed_output_note_ids = self .updated_output_notes - .iter() + .values() .filter_map(|note_record| note_record.is_committed().then_some(note_record.id())); let committed_input_note_ids = self .updated_input_notes - .iter() + .values() .filter_map(|note_record| note_record.is_committed().then_some(note_record.id())); BTreeSet::from_iter(committed_input_note_ids.chain(committed_output_note_ids)) @@ -336,14 +311,52 @@ impl NoteUpdates { pub fn consumed_note_ids(&self) -> BTreeSet { let consumed_output_note_ids = self .updated_output_notes - .iter() + .values() .filter_map(|note_record| note_record.is_consumed().then_some(note_record.id())); let consumed_input_note_ids = self .updated_input_notes - .iter() + .values() .filter_map(|note_record| note_record.is_consumed().then_some(note_record.id())); BTreeSet::from_iter(consumed_input_note_ids.chain(consumed_output_note_ids)) } + + pub fn insert_or_ignore_notes( + &mut self, + input_notes: &Vec, + output_notes: &Vec, + ) { + for note in input_notes { + self.updated_input_notes.entry(note.id()).or_insert(note.clone()); + } + + for note in output_notes { + self.updated_output_notes.entry(note.id()).or_insert(note.clone()); + } + } + + pub fn get_input_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut InputNoteRecord> { + self.updated_input_notes.get_mut(note_id) + } + + pub fn get_output_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut OutputNoteRecord> { + self.updated_output_notes.get_mut(note_id) + } + + pub fn get_input_note_by_nullifier( + &mut self, + nullifier: Nullifier, + ) -> Option<&mut InputNoteRecord> { + self.updated_input_notes.values_mut().find(|note| note.nullifier() == nullifier) + } + + pub fn get_output_note_by_nullifier( + &mut self, + nullifier: Nullifier, + ) -> Option<&mut OutputNoteRecord> { + self.updated_output_notes + .values_mut() + .find(|note| note.nullifier() == Some(nullifier)) + } } diff --git a/crates/rust-client/src/store/sqlite_store/note.rs b/crates/rust-client/src/store/sqlite_store/note.rs index bd6a831ba..ce017c67d 100644 --- a/crates/rust-client/src/store/sqlite_store/note.rs +++ b/crates/rust-client/src/store/sqlite_store/note.rs @@ -587,17 +587,11 @@ pub(crate) fn apply_note_updates_tx( tx: &Transaction, note_updates: &NoteUpdates, ) -> Result<(), StoreError> { - for input_note in - note_updates.new_input_notes().iter().chain(note_updates.updated_input_notes()) - { + for input_note in note_updates.updated_input_notes() { upsert_input_note_tx(tx, input_note)?; } - for output_note in note_updates - .new_output_notes() - .iter() - .chain(note_updates.updated_output_notes()) - { + for output_note in note_updates.updated_output_notes() { upsert_output_note_tx(tx, output_note)?; } diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 1086cfba3..81e01c645 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -94,14 +94,12 @@ impl SqliteStore { pub(super) fn apply_state_sync_step( conn: &mut Connection, state_sync_update: StateSyncUpdate, - block_has_relevant_notes: bool, + _block_has_relevant_notes: bool, ) -> Result<(), StoreError> { let StateSyncUpdate { - block_header, + block_headers, note_updates, transaction_updates, - new_mmr_peaks, - new_authentication_nodes, account_updates, tags_to_remove, } = state_sync_update; @@ -125,10 +123,25 @@ impl SqliteStore { // Update state sync block number const BLOCK_NUMBER_QUERY: &str = "UPDATE state_sync SET block_num = ?"; - tx.execute(BLOCK_NUMBER_QUERY, params![block_header.block_num().as_u32() as i64])?; - - Self::insert_block_header_tx(&tx, block_header, new_mmr_peaks, block_has_relevant_notes)?; + if let Some(max_block_num) = + block_headers.iter().map(|(header, ..)| header.block_num().as_u32()).max() + { + tx.execute(BLOCK_NUMBER_QUERY, params![max_block_num as i64])?; + } + for (block_header, block_has_relevant_notes, new_mmr_peaks, new_authentication_nodes) in + block_headers + { + Self::insert_block_header_tx( + &tx, + block_header, + new_mmr_peaks, + block_has_relevant_notes, + )?; + + // Insert new authentication nodes (inner nodes of the PartialMmr) + Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?; + } // Update notes apply_note_updates_tx(&tx, ¬e_updates)?; @@ -137,9 +150,6 @@ impl SqliteStore { remove_note_tag_tx(&tx, tag)?; } - // Insert new authentication nodes (inner nodes of the PartialMmr) - Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?; - // Mark transactions as committed Self::mark_transactions_as_committed(&tx, transaction_updates.committed_transactions())?; diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index 383d158a1..741bca440 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -190,86 +190,84 @@ impl Client { self.rpc_api.clone(), Box::new({ let store_clone = self.store.clone(); - move |committed_notes, block_header| { - Box::pin(on_note_received(store_clone.clone(), committed_notes, block_header)) + move |note_updates, committed_note, block_header| { + Box::pin(on_note_received( + store_clone.clone(), + note_updates, + committed_note, + block_header, + )) } }), Box::new({ let store_clone = self.store.clone(); - move |transaction_update| { - Box::pin(on_transaction_committed(store_clone.clone(), transaction_update)) + move |note_updates, transaction_update| { + Box::pin(on_transaction_committed( + store_clone.clone(), + note_updates, + transaction_update, + )) } }), Box::new({ let store_clone = self.store.clone(); - move |nullifier_update| { - Box::pin(on_nullifier_received(store_clone.clone(), nullifier_update)) + move |note_updates, nullifier_update| { + Box::pin(on_nullifier_received( + store_clone.clone(), + note_updates, + nullifier_update, + )) } }), ); + // Get current state of the client let current_block_num = self.store.get_sync_height().await?; - let mut total_sync_summary = SyncSummary::new_empty(current_block_num); + let (current_block, has_relevant_notes) = self + .store + .get_block_header_by_num(current_block_num) + .await? + .expect("Current block should be in the store"); - loop { - // Get current state of the client - let current_block_num = self.store.get_sync_height().await?; - let (current_block, has_relevant_notes) = self - .store - .get_block_header_by_num(current_block_num) - .await? - .expect("Current block should be in the store"); + let accounts = self + .store + .get_account_headers() + .await? + .into_iter() + .map(|(acc_header, _)| acc_header) + .collect(); - let accounts = self - .store - .get_account_headers() - .await? - .into_iter() - .map(|(acc_header, _)| acc_header) - .collect(); + let note_tags: Vec = + self.store.get_unique_note_tags().await?.into_iter().collect(); - let note_tags: Vec = - self.store.get_unique_note_tags().await?.into_iter().collect(); + let unspent_nullifiers = self.store.get_unspent_input_note_nullifiers().await?; - let unspent_nullifiers = self.store.get_unspent_input_note_nullifiers().await?; + // Get the sync update from the network + let state_sync_update = state_sync + .sync_state( + current_block, + has_relevant_notes, + self.build_current_partial_mmr(false).await?, + accounts, + note_tags, + unspent_nullifiers, + ) + .await?; - // Get the sync update from the network - let status = state_sync - .sync_state_step( - current_block, - has_relevant_notes, - self.build_current_partial_mmr(false).await?, - accounts, - note_tags, - unspent_nullifiers, - ) - .await?; + let sync_summary: SyncSummary = (&state_sync_update).into(); - let (is_last_block, state_sync_update): (bool, StateSyncUpdate) = match status { - Some(s) => (s.is_last_block(), s.into()), - None => break, - }; + let has_relevant_notes = + self.check_block_relevance(&state_sync_update.note_updates).await?; - let sync_summary: SyncSummary = (&state_sync_update).into(); + // Apply received and computed updates to the store + self.store + .apply_state_sync_step(state_sync_update, has_relevant_notes) + .await + .map_err(ClientError::StoreError)?; - let has_relevant_notes = - self.check_block_relevance(&state_sync_update.note_updates).await?; - - // Apply received and computed updates to the store - self.store - .apply_state_sync_step(state_sync_update, has_relevant_notes) - .await - .map_err(ClientError::StoreError)?; - - total_sync_summary.combine_with(sync_summary); - - if is_last_block { - break; - } - } self.update_mmr_data().await?; - Ok(total_sync_summary) + Ok(sync_summary) } } diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 1b6336bbe..ed54dc2f8 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -26,20 +26,16 @@ use crate::{ // STATE SYNC UPDATE // ================================================================================================ +#[derive(Default)] /// Contains all information needed to apply the update in the store after syncing with the node. pub struct StateSyncUpdate { /// The new block header, returned as part of the /// [StateSyncInfo](crate::rpc::domain::sync::StateSyncInfo) - pub block_header: BlockHeader, + pub block_headers: Vec<(BlockHeader, bool, MmrPeaks, Vec<(InOrderIndex, Digest)>)>, //TODO: move this to a new block update struct /// Information about note changes after the sync. pub note_updates: NoteUpdates, /// Information about transaction changes after the sync. pub transaction_updates: TransactionUpdates, - /// New MMR peaks for the locally tracked MMR of the blockchain. - pub new_mmr_peaks: MmrPeaks, - /// New authentications nodes that are meant to be stored in order to authenticate block - /// headers. - pub new_authentication_nodes: Vec<(InOrderIndex, Digest)>, /// Information abount account changes after the sync. pub account_updates: AccountUpdates, /// Tag records that are no longer relevant. @@ -49,8 +45,13 @@ pub struct StateSyncUpdate { impl From<&StateSyncUpdate> for SyncSummary { fn from(value: &StateSyncUpdate) -> Self { SyncSummary::new( - value.block_header.block_num(), - value.note_updates.new_input_notes().iter().map(|n| n.id()).collect(), + value + .block_headers + .iter() + .map(|(h, ..)| h.block_num()) + .max() + .unwrap_or(0.into()), + vec![], //TODO: get new received notes value.note_updates.committed_note_ids().into_iter().collect(), value.note_updates.consumed_note_ids().into_iter().collect(), value @@ -105,6 +106,7 @@ impl From for StateSyncUpdate { /// that should be queried from the node and start being tracked. pub type OnNoteReceived = Box< dyn Fn( + NoteUpdates, CommittedNote, BlockHeader, ) -> Pin), ClientError>>>>, @@ -115,6 +117,7 @@ pub type OnNoteReceived = Box< /// updates that should be applied to the store as a result of the transaction being committed. pub type OnTransactionCommitted = Box< dyn Fn( + NoteUpdates, TransactionUpdate, ) -> Pin>>>, @@ -127,6 +130,7 @@ pub type OnTransactionCommitted = Box< /// store as a result of the nullifier being received. pub type OnNullifierReceived = Box< dyn Fn( + NoteUpdates, NullifierUpdate, ) -> Pin>>>, @@ -149,6 +153,7 @@ pub struct StateSync { on_transaction_committed: OnTransactionCommitted, /// Callback to be executed when a nullifier is received. on_nullifier_received: OnNullifierReceived, + state_sync_update: StateSyncUpdate, } impl StateSync { @@ -171,6 +176,7 @@ impl StateSync { on_note_received, on_transaction_committed, on_nullifier_received, + state_sync_update: StateSyncUpdate::default(), } } @@ -193,15 +199,15 @@ impl StateSync { /// * `accounts` - The headers of tracked accounts. /// * `note_tags` - The note tags to be used in the sync state request. /// * `unspent_nullifiers` - The nullifiers of tracked notes that haven't been consumed. - pub async fn sync_state_step( - &self, + async fn sync_state_step( + &mut self, current_block: BlockHeader, current_block_has_relevant_notes: bool, - current_partial_mmr: PartialMmr, - accounts: Vec, - note_tags: Vec, - unspent_nullifiers: Vec, - ) -> Result, ClientError> { + current_partial_mmr: &mut PartialMmr, + accounts: &[AccountHeader], + note_tags: &[NoteTag], + unspent_nullifiers: &[Nullifier], + ) -> Result { let current_block_num = current_block.block_num(); let account_ids: Vec = accounts.iter().map(|acc| acc.id()).collect(); @@ -214,25 +220,23 @@ impl StateSync { let response = self .rpc_api - .sync_state(current_block_num, &account_ids, ¬e_tags, &nullifiers_tags) + .sync_state(current_block_num, &account_ids, note_tags, &nullifiers_tags) .await?; // We don't need to continue if the chain has not advanced, there are no new changes if response.block_header.block_num() == current_block_num { - return Ok(None); + return Ok(false); } - let account_updates = - self.account_state_sync(&accounts, &response.account_hash_updates).await?; + self.account_state_sync(accounts, &response.account_hash_updates).await?; - let (note_updates, transaction_updates, tags_to_remove) = self - .note_state_sync( - response.note_inclusions, - response.transactions, - response.nullifiers, - response.block_header, - ) - .await?; + self.note_state_sync( + response.note_inclusions, + response.transactions, + response.nullifiers, + response.block_header, + ) + .await?; let (new_mmr_peaks, new_authentication_nodes) = apply_mmr_changes( current_block, @@ -242,20 +246,50 @@ impl StateSync { ) .await?; - let update = StateSyncUpdate { - block_header: response.block_header, - note_updates, - transaction_updates, + self.state_sync_update.block_headers.push(( + response.block_header, + true, //TODO: check if relevant new_mmr_peaks, new_authentication_nodes, - account_updates, - tags_to_remove, - }; + )); if response.chain_tip == response.block_header.block_num() { - Ok(Some(SyncStatus::SyncedToLastBlock(update))) + return Ok(false); } else { - Ok(Some(SyncStatus::SyncedToBlock(update))) + return Ok(true); + } + } + + pub async fn sync_state( + mut self, + mut current_block: BlockHeader, + mut current_block_has_relevant_notes: bool, + mut current_partial_mmr: PartialMmr, + accounts: Vec, + note_tags: Vec, + unspent_nullifiers: Vec, + ) -> Result { + loop { + if !self + .sync_state_step( + current_block, + current_block_has_relevant_notes, + &mut current_partial_mmr, + &accounts, + ¬e_tags, //TODO: get note tags from notes in the updates + &unspent_nullifiers, //TODO: get nullifiers from notes in the updates + ) + .await? + { + return Ok(self.state_sync_update); + } + + (current_block, current_block_has_relevant_notes, ..) = self + .state_sync_update + .block_headers + .last() + .cloned() + .expect("At least one block header should be present"); } } @@ -271,10 +305,10 @@ impl StateSync { /// * If the account is private it will be marked as mismatched and the client will need to /// handle it (it could be a stale account state or a reason to lock the account). async fn account_state_sync( - &self, + &mut self, accounts: &[AccountHeader], account_hash_updates: &[(AccountId, Digest)], - ) -> Result { + ) -> Result<(), ClientError> { let (public_accounts, private_accounts): (Vec<_>, Vec<_>) = accounts.iter().partition(|account_header| account_header.id().is_public()); @@ -291,7 +325,11 @@ impl StateSync { .cloned() .collect::>(); - Ok(AccountUpdates::new(updated_public_accounts, mismatched_private_accounts)) + self.state_sync_update + .account_updates + .extend(AccountUpdates::new(updated_public_accounts, mismatched_private_accounts)); + + Ok(()) } /// Queries the node for the latest state of the public accounts that don't match the current @@ -336,31 +374,30 @@ impl StateSync { /// * Local tracked transactions that were discarded because the notes that they were processing /// were nullified by an another transaction. async fn note_state_sync( - &self, + &mut self, note_inclusions: Vec, transactions: Vec, mut nullifiers: Vec, block_header: BlockHeader, - ) -> Result<(NoteUpdates, TransactionUpdates, Vec), ClientError> { - let mut note_updates = NoteUpdates::default(); + ) -> Result<(), ClientError> { let mut public_note_ids = vec![]; + let mut note_updates = self.state_sync_update.note_updates.clone(); for committed_note in note_inclusions { - let (new_note_update, new_note_ids) = - (self.on_note_received)(committed_note, block_header).await?; - note_updates.extend(new_note_update); + let (new_note_updates, new_note_ids) = + (self.on_note_received)(note_updates, committed_note, block_header).await?; + note_updates = new_note_updates; public_note_ids.extend(new_note_ids); } let new_public_notes = self.fetch_public_note_details(&public_note_ids, &block_header).await?; - note_updates.extend(NoteUpdates::new(new_public_notes, vec![], vec![], vec![])); + note_updates.insert_or_ignore_notes(&new_public_notes, &vec![]); // We can remove tags from notes that got committed - let tags_to_remove = note_updates + let tags_to_remove: Vec = note_updates .updated_input_notes() - .iter() .filter(|note| note.is_committed()) .map(|note| { NoteTagRecord::with_note_source( @@ -370,32 +407,34 @@ impl StateSync { }) .collect(); - let mut transaction_updates = TransactionUpdates::default(); + self.state_sync_update.tags_to_remove.extend(tags_to_remove); for transaction_update in transactions { - let (new_note_update, new_transaction_update) = - (self.on_transaction_committed)(transaction_update).await?; + let (new_note_updates, new_transaction_update) = + (self.on_transaction_committed)(note_updates, transaction_update).await?; // Remove nullifiers if they were consumed by the transaction nullifiers.retain(|nullifier| { - !new_note_update + !new_note_updates .updated_input_notes() - .iter() .any(|note| note.nullifier() == nullifier.nullifier) }); - note_updates.extend(new_note_update); - transaction_updates.extend(new_transaction_update); + note_updates = new_note_updates; + self.state_sync_update.transaction_updates.extend(new_transaction_update); } for nullifier_update in nullifiers { - let (new_note_update, new_transaction_update) = - (self.on_nullifier_received)(nullifier_update).await?; - note_updates.extend(new_note_update); - transaction_updates.extend(new_transaction_update); + let (new_note_updates, new_transaction_update) = + (self.on_nullifier_received)(note_updates, nullifier_update).await?; + + note_updates = new_note_updates; + self.state_sync_update.transaction_updates.extend(new_transaction_update); } - Ok((note_updates, transaction_updates, tags_to_remove)) + self.state_sync_update.note_updates = note_updates; + + Ok(()) } /// Queries the node for all received notes that aren't being locally tracked in the client. @@ -430,7 +469,7 @@ impl StateSync { pub(crate) async fn apply_mmr_changes( current_block: BlockHeader, current_block_has_relevant_notes: bool, - mut current_partial_mmr: PartialMmr, + current_partial_mmr: &mut PartialMmr, mmr_delta: MmrDelta, ) -> Result<(MmrPeaks, Vec<(InOrderIndex, Digest)>), ClientError> { // First, apply curent_block to the MMR. This is needed as the MMR delta received from the @@ -458,6 +497,7 @@ pub(crate) async fn apply_mmr_changes( /// note ID to be queried from the node so it can be queried from the node and tracked. pub async fn on_note_received( store: Arc, + mut note_updates: NoteUpdates, committed_note: CommittedNote, block_header: BlockHeader, ) -> Result<(NoteUpdates, Vec), ClientError> { @@ -467,45 +507,36 @@ pub async fn on_note_received( committed_note.merkle_path().clone(), )?; - let mut updated_input_notes = vec![]; - let mut updated_output_notes = vec![]; + let mut is_tracked_note = false; let mut new_note_ids = vec![]; - if let Some(mut input_note_record) = store - .get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])) - .await? - .pop() - { + note_updates.insert_or_ignore_notes( + &store.get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])).await?, + &store + .get_output_notes(NoteFilter::List(vec![*committed_note.note_id()])) + .await?, + ); + + if let Some(input_note_record) = note_updates.get_input_note_by_id(committed_note.note_id()) { // The note belongs to our locally tracked set of input notes - let inclusion_proof_received = input_note_record + is_tracked_note = true; + input_note_record .inclusion_proof_received(inclusion_proof.clone(), committed_note.metadata())?; - let block_header_received = input_note_record.block_header_received(block_header)?; - - if inclusion_proof_received || block_header_received { - updated_input_notes.push(input_note_record); - } + input_note_record.block_header_received(block_header)?; } - if let Some(mut output_note_record) = store - .get_output_notes(NoteFilter::List(vec![*committed_note.note_id()])) - .await? - .pop() - { + if let Some(output_note_record) = note_updates.get_output_note_by_id(committed_note.note_id()) { // The note belongs to our locally tracked set of output notes - if output_note_record.inclusion_proof_received(inclusion_proof.clone())? { - updated_output_notes.push(output_note_record); - } + is_tracked_note = true; + output_note_record.inclusion_proof_received(inclusion_proof.clone())?; } - if updated_input_notes.is_empty() && updated_output_notes.is_empty() { + if !is_tracked_note { // The note is public and we are not tracking it, push to the list of IDs to query new_note_ids.push(*committed_note.note_id()); } - Ok(( - NoteUpdates::new(vec![], vec![], updated_input_notes, updated_output_notes), - new_note_ids, - )) + Ok((note_updates, new_note_ids)) } /// Default implementation of the [OnTransactionCommitted] callback. It queries the store for the @@ -513,6 +544,7 @@ pub async fn on_note_received( /// also returns the committed transaction update to be applied to the store. pub async fn on_transaction_committed( store: Arc, + mut note_updates: NoteUpdates, transaction_update: TransactionUpdate, ) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { // TODO: This could be improved if we add a filter to get only notes that are being processed by @@ -531,29 +563,29 @@ pub async fn on_transaction_committed( )) .await?; - let mut updated_input_notes = vec![]; - let mut updated_output_notes = vec![]; - for mut input_note_record in consumed_input_notes { - if input_note_record.transaction_committed( + note_updates.insert_or_ignore_notes(&consumed_input_notes, &consumed_output_notes); + + for store_note in consumed_input_notes { + let input_note_record = note_updates + .get_input_note_by_id(&store_note.id()) + .expect("Input note should be present in the note updates after being inserted"); + + input_note_record.transaction_committed( transaction_update.transaction_id, transaction_update.block_num, - )? { - updated_input_notes.push(input_note_record); - } + )?; } - for mut output_note_record in consumed_output_notes { + for store_note in consumed_output_notes { // SAFETY: Output notes were queried from a nullifier list and should have a nullifier - let nullifier = output_note_record.nullifier().unwrap(); - if output_note_record.nullifier_received(nullifier, transaction_update.block_num)? { - updated_output_notes.push(output_note_record); - } + let nullifier = store_note.nullifier().unwrap(); + let output_note_record = note_updates + .get_output_note_by_id(&store_note.id()) + .expect("Output note should be present in the note updates after being inserted"); + output_note_record.nullifier_received(nullifier, transaction_update.block_num)?; } - Ok(( - NoteUpdates::new(vec![], vec![], updated_input_notes, updated_output_notes), - TransactionUpdates::new(vec![transaction_update], vec![]), - )) + Ok((note_updates, TransactionUpdates::new(vec![transaction_update], vec![]))) } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes @@ -561,16 +593,22 @@ pub async fn on_transaction_committed( /// transactions that should be discarded as they weren't committed when the nullifier was received. pub async fn on_nullifier_received( store: Arc, + mut note_updates: NoteUpdates, nullifier_update: NullifierUpdate, ) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { let mut discarded_transactions = vec![]; - let mut updated_input_notes = vec![]; - let mut updated_output_notes = vec![]; - if let Some(mut input_note_record) = store - .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await? - .pop() + note_updates.insert_or_ignore_notes( + &store + .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) + .await?, + &store + .get_output_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) + .await?, + ); + + if let Some(input_note_record) = + note_updates.get_input_note_by_nullifier(nullifier_update.nullifier) { if input_note_record.is_processing() { discarded_transactions.push( @@ -580,27 +618,16 @@ pub async fn on_nullifier_received( ); } - if input_note_record - .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)? - { - updated_input_notes.push(input_note_record); - } + input_note_record + .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } - if let Some(mut output_note_record) = store - .get_output_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await? - .pop() + if let Some(output_note_record) = + note_updates.get_output_note_by_nullifier(nullifier_update.nullifier) { - if output_note_record - .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)? - { - updated_output_notes.push(output_note_record); - } + output_note_record + .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)?; } - Ok(( - NoteUpdates::new(vec![], vec![], updated_input_notes, updated_output_notes), - TransactionUpdates::new(vec![], discarded_transactions), - )) + Ok((note_updates, TransactionUpdates::new(vec![], discarded_transactions))) } diff --git a/crates/rust-client/src/transaction/mod.rs b/crates/rust-client/src/transaction/mod.rs index 6ecf951eb..5bd4b1672 100644 --- a/crates/rust-client/src/transaction/mod.rs +++ b/crates/rust-client/src/transaction/mod.rs @@ -363,10 +363,8 @@ impl TransactionStoreUpdate { executed_transaction, updated_account, note_updates: NoteUpdates::new( - created_input_notes, + [created_input_notes, updated_input_notes].concat(), created_output_notes, - updated_input_notes, - vec![], ), new_tags, } From e61c958d913476fd2f093c0b699c70ddabde02a3 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Wed, 29 Jan 2025 16:57:46 -0300 Subject: [PATCH 02/14] feat: add block relevance in `StateSyncUpdate` --- bin/miden-cli/src/commands/sync.rs | 3 +- crates/rust-client/src/store/mod.rs | 10 +- .../rust-client/src/store/sqlite_store/mod.rs | 8 +- .../src/store/sqlite_store/sync.rs | 23 ++- crates/rust-client/src/sync/block_header.rs | 42 ++---- crates/rust-client/src/sync/mod.rs | 42 +++--- crates/rust-client/src/sync/state_sync.rs | 140 ++++++++++++------ crates/web-client/src/models/sync_summary.rs | 4 - tests/integration/main.rs | 4 +- 9 files changed, 149 insertions(+), 127 deletions(-) diff --git a/bin/miden-cli/src/commands/sync.rs b/bin/miden-cli/src/commands/sync.rs index da5b763d8..cc6f1e7b8 100644 --- a/bin/miden-cli/src/commands/sync.rs +++ b/bin/miden-cli/src/commands/sync.rs @@ -12,8 +12,7 @@ impl SyncCmd { let new_details = client.sync_state().await?; println!("State synced to block {}", new_details.block_num); - println!("New public notes: {}", new_details.received_notes.len()); - println!("Tracked notes updated: {}", new_details.committed_notes.len()); + println!("Committed notes: {}", new_details.committed_notes.len()); println!("Tracked notes consumed: {}", new_details.consumed_notes.len()); println!("Tracked accounts updated: {}", new_details.updated_accounts.len()); println!("Locked accounts: {}", new_details.locked_accounts.len()); diff --git a/crates/rust-client/src/store/mod.rs b/crates/rust-client/src/store/mod.rs index 0872074a7..463fa7b77 100644 --- a/crates/rust-client/src/store/mod.rs +++ b/crates/rust-client/src/store/mod.rs @@ -327,15 +327,7 @@ pub trait Store: Send + Sync { /// locally does not match the `StateSyncUpdate` information, the account may be locked. /// - Storing new MMR authentication nodes. /// - Updating the tracked public accounts. - /// - /// A [StateSyncUpdate] corresponds to an update from a single `SyncState` RPC call. For the - /// client to be up to date against the current chain tip, multiple calls may be performed, and - /// so multiple store updates could happen sequentially. - async fn apply_state_sync_step( - &self, - state_sync_update: StateSyncUpdate, - new_block_has_relevant_notes: bool, - ) -> Result<(), StoreError>; + async fn apply_state_sync(&self, state_sync_update: StateSyncUpdate) -> Result<(), StoreError>; } // CHAIN MMR NODE FILTER diff --git a/crates/rust-client/src/store/sqlite_store/mod.rs b/crates/rust-client/src/store/sqlite_store/mod.rs index 576cb171a..92d9715ac 100644 --- a/crates/rust-client/src/store/sqlite_store/mod.rs +++ b/crates/rust-client/src/store/sqlite_store/mod.rs @@ -144,13 +144,9 @@ impl Store for SqliteStore { self.interact_with_connection(SqliteStore::get_sync_height).await } - async fn apply_state_sync_step( - &self, - state_sync_update: StateSyncUpdate, - block_has_relevant_notes: bool, - ) -> Result<(), StoreError> { + async fn apply_state_sync(&self, state_sync_update: StateSyncUpdate) -> Result<(), StoreError> { self.interact_with_connection(move |conn| { - SqliteStore::apply_state_sync_step(conn, state_sync_update, block_has_relevant_notes) + SqliteStore::apply_state_sync(conn, state_sync_update) }) .await } diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 81e01c645..655b52127 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -91,13 +91,13 @@ impl SqliteStore { .expect("state sync block number exists") } - pub(super) fn apply_state_sync_step( + pub(super) fn apply_state_sync( conn: &mut Connection, state_sync_update: StateSyncUpdate, - _block_has_relevant_notes: bool, ) -> Result<(), StoreError> { let StateSyncUpdate { - block_headers, + block_num, + block_updates, note_updates, transaction_updates, account_updates, @@ -123,25 +123,20 @@ impl SqliteStore { // Update state sync block number const BLOCK_NUMBER_QUERY: &str = "UPDATE state_sync SET block_num = ?"; - if let Some(max_block_num) = - block_headers.iter().map(|(header, ..)| header.block_num().as_u32()).max() - { - tx.execute(BLOCK_NUMBER_QUERY, params![max_block_num as i64])?; - } + tx.execute(BLOCK_NUMBER_QUERY, params![block_num.as_u64() as i64])?; - for (block_header, block_has_relevant_notes, new_mmr_peaks, new_authentication_nodes) in - block_headers - { + for (block_header, block_has_relevant_notes, new_mmr_peaks) in block_updates.block_headers { Self::insert_block_header_tx( &tx, block_header, new_mmr_peaks, block_has_relevant_notes, )?; - - // Insert new authentication nodes (inner nodes of the PartialMmr) - Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?; } + + // Insert new authentication nodes (inner nodes of the PartialMmr) + Self::insert_chain_mmr_nodes_tx(&tx, &block_updates.new_authentication_nodes)?; + // Update notes apply_note_updates_tx(&tx, ¬e_updates)?; diff --git a/crates/rust-client/src/sync/block_header.rs b/crates/rust-client/src/sync/block_header.rs index 7d63224a7..a60cd9f22 100644 --- a/crates/rust-client/src/sync/block_header.rs +++ b/crates/rust-client/src/sync/block_header.rs @@ -8,13 +8,24 @@ use miden_objects::{ }; use tracing::warn; -use super::NoteUpdates; use crate::{ - note::NoteScreener, store::{ChainMmrNodeFilter, NoteFilter, StoreError}, Client, ClientError, }; +#[derive(Debug, Clone, Default)] +pub struct BlockUpdates { + pub block_headers: Vec<(BlockHeader, bool, MmrPeaks)>, + pub new_authentication_nodes: Vec<(InOrderIndex, Digest)>, +} + +impl BlockUpdates { + pub fn extend(&mut self, other: BlockUpdates) { + self.block_headers.extend(other.block_headers); + self.new_authentication_nodes.extend(other.new_authentication_nodes); + } +} + /// Network information management methods. impl Client { /// Updates committed notes with no MMR data. These could be notes that were @@ -73,33 +84,6 @@ impl Client { // HELPERS // -------------------------------------------------------------------------------------------- - /// Checks the relevance of the block by verifying if any of the input notes in the block are - /// relevant to the client. If any of the notes are relevant, the function returns `true`. - pub(crate) async fn check_block_relevance( - &self, - note_updates: &NoteUpdates, - ) -> Result { - // We'll only do the check for either incoming public notes or expected input notes as - // output notes are not really candidates to be consumed here. - - let note_screener = NoteScreener::new(self.store.clone()); - - // Find all relevant Input Notes using the note checker - for input_note in note_updates.committed_input_notes() { - // TODO: Map the below error into a better representation (ie, we expected to be able - // to convert here) - if !note_screener - .check_relevance(&input_note.try_into().map_err(ClientError::NoteRecordError)?) - .await? - .is_empty() - { - return Ok(true); - } - } - - Ok(false) - } - /// Builds the current store view of the chain's [PartialMmr]. Because we want to add all new /// authentication nodes that could come from applying the MMR updates, we need to track all /// known leaves thus far. diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index 741bca440..f5d42b65b 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -38,7 +38,6 @@ //! let sync_summary: SyncSummary = client.sync_state().await?; //! //! println!("Synced up to block number: {}", sync_summary.block_num); -//! println!("Received notes: {}", sync_summary.received_notes.len()); //! println!("Committed notes: {}", sync_summary.committed_notes.len()); //! println!("Consumed notes: {}", sync_summary.consumed_notes.len()); //! println!("Updated accounts: {}", sync_summary.updated_accounts.len()); @@ -66,7 +65,7 @@ use miden_objects::{ transaction::TransactionId, }; -use crate::{note::NoteUpdates, Client, ClientError}; +use crate::{Client, ClientError}; mod block_header; @@ -75,17 +74,15 @@ pub use tag::{NoteTagRecord, NoteTagSource}; mod state_sync; pub use state_sync::{ - on_note_received, on_nullifier_received, on_transaction_committed, OnNoteReceived, - OnNullifierReceived, OnTransactionCommitted, StateSync, StateSyncUpdate, SyncStatus, + on_block_received, on_note_received, on_nullifier_received, on_transaction_committed, + OnNoteReceived, OnNullifierReceived, OnTransactionCommitted, StateSync, StateSyncUpdate, }; /// Contains stats about the sync operation. pub struct SyncSummary { /// Block number up to which the client has been synced. pub block_num: BlockNumber, - /// IDs of new notes received. - pub received_notes: Vec, - /// IDs of tracked notes that received inclusion proofs. + /// IDs of notes that have been committed. pub committed_notes: Vec, /// IDs of notes that have been consumed. pub consumed_notes: Vec, @@ -100,7 +97,6 @@ pub struct SyncSummary { impl SyncSummary { pub fn new( block_num: BlockNumber, - received_notes: Vec, committed_notes: Vec, consumed_notes: Vec, updated_accounts: Vec, @@ -109,7 +105,6 @@ impl SyncSummary { ) -> Self { Self { block_num, - received_notes, committed_notes, consumed_notes, updated_accounts, @@ -121,7 +116,6 @@ impl SyncSummary { pub fn new_empty(block_num: BlockNumber) -> Self { Self { block_num, - received_notes: vec![], committed_notes: vec![], consumed_notes: vec![], updated_accounts: vec![], @@ -131,8 +125,7 @@ impl SyncSummary { } pub fn is_empty(&self) -> bool { - self.received_notes.is_empty() - && self.committed_notes.is_empty() + self.committed_notes.is_empty() && self.consumed_notes.is_empty() && self.updated_accounts.is_empty() && self.locked_accounts.is_empty() @@ -140,7 +133,6 @@ impl SyncSummary { pub fn combine_with(&mut self, mut other: Self) { self.block_num = max(self.block_num, other.block_num); - self.received_notes.append(&mut other.received_notes); self.committed_notes.append(&mut other.committed_notes); self.consumed_notes.append(&mut other.consumed_notes); self.updated_accounts.append(&mut other.updated_accounts); @@ -219,6 +211,25 @@ impl Client { )) } }), + Box::new({ + let store_clone = self.store.clone(); + move |new_block_header, + note_updates, + current_block_header, + current_block_has_relevant_notes, + current_partial_mmr, + mmr_delta| { + Box::pin(on_block_received( + store_clone.clone(), + new_block_header, + note_updates, + current_block_header, + current_block_has_relevant_notes, + current_partial_mmr, + mmr_delta, + )) + } + }), ); // Get current state of the client @@ -256,12 +267,9 @@ impl Client { let sync_summary: SyncSummary = (&state_sync_update).into(); - let has_relevant_notes = - self.check_block_relevance(&state_sync_update.note_updates).await?; - // Apply received and computed updates to the store self.store - .apply_state_sync_step(state_sync_update, has_relevant_notes) + .apply_state_sync(state_sync_update) .await .map_err(ClientError::StoreError)?; diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index ed54dc2f8..82bc9812e 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -3,17 +3,17 @@ use core::{future::Future, pin::Pin}; use miden_objects::{ account::{Account, AccountHeader, AccountId}, - block::BlockHeader, + block::{BlockHeader, BlockNumber}, crypto::merkle::{InOrderIndex, MmrDelta, MmrPeaks, PartialMmr}, note::{NoteId, NoteInclusionProof, NoteTag, Nullifier}, Digest, }; use tracing::info; -use super::{get_nullifier_prefix, NoteTagRecord, SyncSummary}; +use super::{block_header::BlockUpdates, get_nullifier_prefix, NoteTagRecord, SyncSummary}; use crate::{ account::AccountUpdates, - note::NoteUpdates, + note::{NoteScreener, NoteUpdates}, rpc::{ domain::{note::CommittedNote, nullifier::NullifierUpdate, transaction::TransactionUpdate}, NodeRpcClient, @@ -29,9 +29,10 @@ use crate::{ #[derive(Default)] /// Contains all information needed to apply the update in the store after syncing with the node. pub struct StateSyncUpdate { + pub block_num: BlockNumber, /// The new block header, returned as part of the /// [StateSyncInfo](crate::rpc::domain::sync::StateSyncInfo) - pub block_headers: Vec<(BlockHeader, bool, MmrPeaks, Vec<(InOrderIndex, Digest)>)>, //TODO: move this to a new block update struct + pub block_updates: BlockUpdates, /// Information about note changes after the sync. pub note_updates: NoteUpdates, /// Information about transaction changes after the sync. @@ -45,13 +46,7 @@ pub struct StateSyncUpdate { impl From<&StateSyncUpdate> for SyncSummary { fn from(value: &StateSyncUpdate) -> Self { SyncSummary::new( - value - .block_headers - .iter() - .map(|(h, ..)| h.block_num()) - .max() - .unwrap_or(0.into()), - vec![], //TODO: get new received notes + value.block_num, value.note_updates.committed_note_ids().into_iter().collect(), value.note_updates.consumed_note_ids().into_iter().collect(), value @@ -76,27 +71,6 @@ impl From<&StateSyncUpdate> for SyncSummary { } } -/// Gives information about the status of the sync process after a step. -pub enum SyncStatus { - SyncedToLastBlock(StateSyncUpdate), - SyncedToBlock(StateSyncUpdate), -} - -impl SyncStatus { - pub fn is_last_block(&self) -> bool { - matches!(self, SyncStatus::SyncedToLastBlock(_)) - } -} - -impl From for StateSyncUpdate { - fn from(value: SyncStatus) -> StateSyncUpdate { - match value { - SyncStatus::SyncedToLastBlock(update) => update, - SyncStatus::SyncedToBlock(update) => update, - } - } -} - // SYNC CALLBACKS // ================================================================================================ @@ -136,6 +110,17 @@ pub type OnNullifierReceived = Box< -> Pin>>>, >; +pub type OnBlockHeaderReceived = Box< + dyn Fn( + BlockHeader, + NoteUpdates, + BlockHeader, + bool, + PartialMmr, + MmrDelta, + ) -> Pin>>>, +>; + // STATE SYNC // ================================================================================================ @@ -153,6 +138,7 @@ pub struct StateSync { on_transaction_committed: OnTransactionCommitted, /// Callback to be executed when a nullifier is received. on_nullifier_received: OnNullifierReceived, + on_block_received: OnBlockHeaderReceived, state_sync_update: StateSyncUpdate, } @@ -170,12 +156,14 @@ impl StateSync { on_note_received: OnNoteReceived, on_transaction_committed: OnTransactionCommitted, on_nullifier_received: OnNullifierReceived, + on_block_received: OnBlockHeaderReceived, ) -> Self { Self { rpc_api, on_note_received, on_transaction_committed, on_nullifier_received, + on_block_received, state_sync_update: StateSyncUpdate::default(), } } @@ -223,6 +211,8 @@ impl StateSync { .sync_state(current_block_num, &account_ids, note_tags, &nullifiers_tags) .await?; + self.state_sync_update.block_num = response.block_header.block_num(); + // We don't need to continue if the chain has not advanced, there are no new changes if response.block_header.block_num() == current_block_num { return Ok(false); @@ -238,25 +228,23 @@ impl StateSync { ) .await?; - let (new_mmr_peaks, new_authentication_nodes) = apply_mmr_changes( + let (new_block_updates, new_partial_mmr) = (self.on_block_received)( + response.block_header, + self.state_sync_update.note_updates.clone(), current_block, current_block_has_relevant_notes, - current_partial_mmr, + current_partial_mmr.clone(), response.mmr_delta, ) .await?; - self.state_sync_update.block_headers.push(( - response.block_header, - true, //TODO: check if relevant - new_mmr_peaks, - new_authentication_nodes, - )); + self.state_sync_update.block_updates.extend(new_block_updates); + *current_partial_mmr = new_partial_mmr; if response.chain_tip == response.block_header.block_num() { - return Ok(false); + Ok(false) } else { - return Ok(true); + Ok(true) } } @@ -286,6 +274,7 @@ impl StateSync { (current_block, current_block_has_relevant_notes, ..) = self .state_sync_update + .block_updates .block_headers .last() .cloned() @@ -466,7 +455,7 @@ impl StateSync { /// Applies changes to the current MMR structure, returns the updated [MmrPeaks] and the /// authentication nodes for leaves we track. -pub(crate) async fn apply_mmr_changes( +async fn apply_mmr_changes( current_block: BlockHeader, current_block_has_relevant_notes: bool, current_partial_mmr: &mut PartialMmr, @@ -631,3 +620,68 @@ pub async fn on_nullifier_received( Ok((note_updates, TransactionUpdates::new(vec![], discarded_transactions))) } + +pub async fn on_block_received( + store: Arc, + new_block_header: BlockHeader, + note_updates: NoteUpdates, + current_block_header: BlockHeader, + current_block_has_relevant_notes: bool, + mut current_partial_mmr: PartialMmr, + mmr_delta: MmrDelta, +) -> Result<(BlockUpdates, PartialMmr), ClientError> { + let (mmr_peaks, new_authentication_nodes) = apply_mmr_changes( + current_block_header, + current_block_has_relevant_notes, + &mut current_partial_mmr, + mmr_delta, + ) + .await?; + + let block_relevance = + check_block_relevance(store.clone(), new_block_header.block_num(), note_updates).await?; + + Ok(( + BlockUpdates { + block_headers: vec![(new_block_header, block_relevance, mmr_peaks)], + new_authentication_nodes, + }, + current_partial_mmr, + )) +} + +/// Checks the relevance of the block by verifying if any of the input notes in the block are +/// relevant to the client. If any of the notes are relevant, the function returns `true`. +pub(crate) async fn check_block_relevance( + store: Arc, + new_block_number: BlockNumber, + note_updates: NoteUpdates, +) -> Result { + // We'll only do the check for either incoming public notes or expected input notes as + // output notes are not really candidates to be consumed here. + + let note_screener = NoteScreener::new(store); + + // Find all relevant Input Notes using the note checker + for input_note in note_updates.committed_input_notes() { + if input_note + .inclusion_proof() + .is_some_and(|proof| proof.location().block_num() != new_block_number) + { + // This note wasn't received in the current block, so it's not relevant + continue; + } + + // TODO: Map the below error into a better representation (ie, we expected to be able + // to convert here) + if !note_screener + .check_relevance(&input_note.try_into().map_err(ClientError::NoteRecordError)?) + .await? + .is_empty() + { + return Ok(true); + } + } + + Ok(false) +} diff --git a/crates/web-client/src/models/sync_summary.rs b/crates/web-client/src/models/sync_summary.rs index 34d1bf4e1..67ac51fda 100644 --- a/crates/web-client/src/models/sync_summary.rs +++ b/crates/web-client/src/models/sync_summary.rs @@ -12,10 +12,6 @@ impl SyncSummary { self.0.block_num.as_u32() } - pub fn received_notes(&self) -> Vec { - self.0.received_notes.iter().map(|note_id| note_id.into()).collect() - } - pub fn committed_notes(&self) -> Vec { self.0.committed_notes.iter().map(|note_id| note_id.into()).collect() } diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 21c49ed83..1d8e80e8b 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -850,8 +850,7 @@ async fn test_sync_detail_values() { // Second client sync should have new note let new_details = client2.sync_state().await.unwrap(); - assert_eq!(new_details.received_notes.len(), 1); - assert_eq!(new_details.committed_notes.len(), 0); + assert_eq!(new_details.committed_notes.len(), 1); assert_eq!(new_details.consumed_notes.len(), 0); assert_eq!(new_details.updated_accounts.len(), 0); @@ -861,7 +860,6 @@ async fn test_sync_detail_values() { // First client sync should have a new nullifier as the note was consumed let new_details = client1.sync_state().await.unwrap(); - assert_eq!(new_details.received_notes.len(), 0); assert_eq!(new_details.committed_notes.len(), 0); assert_eq!(new_details.consumed_notes.len(), 1); } From 7fb99c2309b795067dcf64c266876583647a5a6f Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 12:07:07 -0300 Subject: [PATCH 03/14] fix: web client build --- Cargo.lock | 119 +++++++++++------- crates/rust-client/src/note/mod.rs | 7 +- .../src/store/web_store/js/sync.js | 40 ++++-- crates/rust-client/src/store/web_store/mod.rs | 8 +- .../src/store/web_store/note/utils.rs | 10 +- .../src/store/web_store/sync/js_bindings.rs | 9 +- .../src/store/web_store/sync/mod.rs | 33 +++-- crates/web-client/package-lock.json | 4 +- 8 files changed, 145 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ac21401d0..be3e4ae55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,9 +328,9 @@ dependencies = [ [[package]] name = "bstr" -version = "1.11.1" +version = "1.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "786a307d683a5bf92e6fd5fd69a7eb613751668d1d8d67d802846dfe367c62c8" +checksum = "531a9155a481e2ee699d4f98f43c0ca4ff8ee1bfd55c31e9e98fb29d2b176fe0" dependencies = [ "memchr", "regex-automata 0.4.9", @@ -339,9 +339,9 @@ dependencies = [ [[package]] name = "bumpalo" -version = "3.16.0" +version = "3.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" +checksum = "1628fb46dfa0b37568d12e5edd512553eccf6a22a78e8bde00bb4aed84d5bdbf" [[package]] name = "bytemuck" @@ -474,9 +474,9 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "cpufeatures" -version = "0.2.16" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "16b80225097f2e5ae4e7179dd2266824648f3e2f49d9134d584b76389d31c4c3" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" dependencies = [ "libc", ] @@ -825,10 +825,22 @@ dependencies = [ "cfg-if", "js-sys", "libc", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "wasm-bindgen", ] +[[package]] +name = "getrandom" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43a49c392881ce6d5c3b8cb70f98717b7c07aabbdff06687b9030dbfbe2725f8" +dependencies = [ + "cfg-if", + "libc", + "wasi 0.13.3+wasi-0.2.2", + "windows-targets 0.52.6", +] + [[package]] name = "gimli" version = "0.31.1" @@ -837,9 +849,9 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" [[package]] name = "glob" -version = "0.3.1" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" [[package]] name = "h2" @@ -944,9 +956,9 @@ dependencies = [ [[package]] name = "httparse" -version = "1.9.5" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d71d3574edd2771538b901e6549113b4006ece66150fb69c0fb6d9a2adae946" +checksum = "f2d708df4e7140240a16cd6ab0ab65c972d7433ab77819ea693fde9c43811e2a" [[package]] name = "httpdate" @@ -956,9 +968,9 @@ checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" [[package]] name = "hyper" -version = "1.5.2" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "256fb8d4bd6413123cc9d91832d78325c48ff41677595be797d90f42969beae0" +checksum = "cc2b571658e38e0c01b1fdca3bbbe93c00d3d71693ff2770043f8c29bc7d6f80" dependencies = [ "bytes", "futures-channel", @@ -1426,9 +1438,9 @@ dependencies = [ [[package]] name = "miden-crypto" -version = "0.13.1" +version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a06bf3ad2a85f3f8f0da73b6357c77e482b1ceb36cacda8a2d85caae3bd1f702" +checksum = "1945918276152bd9b8e8434643ad24d4968e075b68a5ed03927b53ac75490a79" dependencies = [ "blake3", "cc", @@ -1511,11 +1523,11 @@ dependencies = [ [[package]] name = "miden-objects" -version = "0.7.1" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4622f71888d4641577d145dd561165824b9b9293fd0be2306326f11bcf39e67d" +checksum = "8fe3f10d0e3787176f0803be2ecb4646f3a17fe10af45a50736c8d079a3c94d8" dependencies = [ - "getrandom", + "getrandom 0.2.15", "miden-assembly", "miden-core", "miden-crypto", @@ -1577,9 +1589,9 @@ dependencies = [ [[package]] name = "miden-rpc-proto" -version = "0.7.0" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c2edb867ffc4cd908bb98b48f326016e36158129faba39aef574a0f33433b60" +checksum = "e68f00e0e97ba6bc65896e5ca2bb0995d59b14fd8f0389916dc2ee792f353f50" [[package]] name = "miden-stdlib" @@ -1674,7 +1686,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2886843bf800fba2e3377cff24abf6379b4c4d5c6681eaf9ea5b0d15090450bd" dependencies = [ "libc", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "windows-sys 0.52.0", ] @@ -1901,18 +1913,18 @@ dependencies = [ [[package]] name = "pin-project" -version = "1.1.7" +version = "1.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be57f64e946e500c8ee36ef6331845d40a93055567ec57e8fae13efd33759b95" +checksum = "1e2ec53ad785f4d35dac0adea7f7dc6f1bb277ad84a680c7afefeae05d1f5916" dependencies = [ "pin-project-internal", ] [[package]] name = "pin-project-internal" -version = "1.1.7" +version = "1.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c0f5fad0874fc7abcd4d750e76917eaebbecaa2c20bde22e1dbeeba8beb758c" +checksum = "d56a66c0c55993aa927429d0f8a0abfd74f084e4d9c192cffed01e418d83eefb" dependencies = [ "proc-macro2", "quote", @@ -1921,9 +1933,9 @@ dependencies = [ [[package]] name = "pin-project-lite" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "915a1e146535de9163f3987b8944ed8cf49a18bb0056bcebcdcece385cece4ff" +checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" [[package]] name = "pin-utils" @@ -2148,7 +2160,7 @@ version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" dependencies = [ - "getrandom", + "getrandom 0.2.15", ] [[package]] @@ -2195,7 +2207,7 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" dependencies = [ - "getrandom", + "getrandom 0.2.15", "libredox", "thiserror 1.0.69", ] @@ -2303,9 +2315,9 @@ checksum = "f7c45b9784283f1b2e7fb61b42047c2fd678ef0960d4f6f1eba131594cc369d4" [[package]] name = "ryu" -version = "1.0.18" +version = "1.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3cb5ba0dc43242ce17de99c180e96db90b235b8a9fdc9543c96d2209116bd9f" +checksum = "6ea1a2d0a644769cc99faa24c3ad26b379b786fe7c36fd3c546254801650e6dd" [[package]] name = "same-file" @@ -2354,9 +2366,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "serde" -version = "1.0.216" +version = "1.0.217" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b9781016e935a97e8beecf0c933758c97a5520d32930e460142b4cd80c6338e" +checksum = "02fc4265df13d6fa1d00ecff087228cc0a2b5f3c0e87e258d8b94a156e984c70" dependencies = [ "serde_derive", ] @@ -2374,9 +2386,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.216" +version = "1.0.217" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46f859dbbf73865c6627ed570e78961cd3ac92407a2d117204c49232485da55e" +checksum = "5a9bf7cf98d04a2b28aead066b7496853d4779c9cc183c440dbac457641e19a0" dependencies = [ "proc-macro2", "quote", @@ -2385,9 +2397,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.137" +version = "1.0.138" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "930cfb6e6abf99298aaad7d29abbef7a9999a9a8806a40088f55f0dcec03146b" +checksum = "d434192e7da787e94a6ea7e9670b26a036d0ca41e0b7efb2676dd32bae872949" dependencies = [ "itoa", "memchr", @@ -2565,12 +2577,13 @@ checksum = "42a4d50cdb458045afc8131fd91b64904da29548bcb63c7236e0844936c13078" [[package]] name = "tempfile" -version = "3.14.0" +version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28cce251fcbc87fac86a866eeb0d6c2d536fc16d06f184bb61aeae11aa4cee0c" +checksum = "38c246215d7d24f48ae091a2902398798e05d978b24315d6efbc00ede9a8bb91" dependencies = [ "cfg-if", "fastrand", + "getrandom 0.3.1", "once_cell", "rustix", "windows-sys 0.59.0", @@ -2992,9 +3005,9 @@ dependencies = [ [[package]] name = "unicode-ident" -version = "1.0.15" +version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11cd88e12b17c6494200a9c1b683a04fcac9573ed74cd1b62aeb2727c5592243" +checksum = "a210d160f08b701c8721ba1c726c11662f877ea6b7094007e1ca9a1041945034" [[package]] name = "unicode-linebreak" @@ -3032,7 +3045,7 @@ version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b3758f5e68192bb96cc8f9b7e2c2cfdabb435499a28499a42f8f984092adad4b" dependencies = [ - "getrandom", + "getrandom 0.2.15", "serde", ] @@ -3097,6 +3110,15 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "wasi" +version = "0.13.3+wasi-0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26816d2e1a4a36a2940b96c5296ce403917633dff8f3440e9b236ed6f6bacad2" +dependencies = [ + "wit-bindgen-rt", +] + [[package]] name = "wasm-bindgen" version = "0.2.100" @@ -3447,9 +3469,9 @@ checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" -version = "0.6.24" +version = "0.6.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d71a593cc5c42ad7876e2c1fda56f314f3754c084128833e64f1345ff8a03a" +checksum = "ad699df48212c6cc6eb4435f35500ac6fd3b9913324f938aea302022ce19d310" dependencies = [ "memchr", ] @@ -3556,6 +3578,15 @@ dependencies = [ "winter-utils", ] +[[package]] +name = "wit-bindgen-rt" +version = "0.33.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3268f3d866458b787f390cf61f4bbb563b922d091359f9608842999eaee3943c" +dependencies = [ + "bitflags", +] + [[package]] name = "yansi" version = "1.0.1" diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index d26dad6fa..992f626b1 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -56,8 +56,11 @@ //! For more details on the API and error handling, see the documentation for the specific functions //! and types in this module. -use alloc::{collections::BTreeSet, string::ToString, vec::Vec}; -use std::collections::BTreeMap; +use alloc::{ + collections::{BTreeMap, BTreeSet}, + string::ToString, + vec::Vec, +}; use miden_lib::transaction::TransactionKernel; use miden_objects::{account::AccountId, crypto::rand::FeltRng}; diff --git a/crates/rust-client/src/store/web_store/js/sync.js b/crates/rust-client/src/store/web_store/js/sync.js index 2a633ac61..ed7cafd03 100644 --- a/crates/rust-client/src/store/web_store/js/sync.js +++ b/crates/rust-client/src/store/web_store/js/sync.js @@ -80,8 +80,9 @@ export async function removeNoteTag(tag, source_note_id, source_account_id) { export async function applyStateSync( blockNum, - blockHeader, - chainMmrPeaks, + newBlockHeadersAsFlattenedVec, + newBlockNums, + chainMmrPeaksAsFlattenedVec, hasClientNotes, nodeIndexes, nodes, @@ -89,6 +90,11 @@ export async function applyStateSync( committedTransactionIds, transactionBlockNums ) { + const newBlockHeaders = reconstructFlattenedVec( + newBlockHeadersAsFlattenedVec + ); + const chainMmrPeaks = reconstructFlattenedVec(chainMmrPeaksAsFlattenedVec); + return db.transaction( "rw", stateSync, @@ -100,13 +106,15 @@ export async function applyStateSync( tags, async (tx) => { await updateSyncHeight(tx, blockNum); - await updateBlockHeader( - tx, - blockNum, - blockHeader, - chainMmrPeaks, - hasClientNotes - ); + for (let i = 0; i < newBlockHeaders.length; i++) { + await updateBlockHeader( + tx, + newBlockNums[i], + newBlockHeaders[i], + chainMmrPeaks[i], + hasClientNotes[i] + ); + } await updateChainMmrNodes(tx, nodeIndexes, nodes); await updateCommittedNoteTags(tx, inputNoteIds); await updateCommittedTransactions( @@ -232,3 +240,17 @@ function uint8ArrayToBase64(bytes) { ); return btoa(binary); } + +// Helper function to reconstruct arrays from flattened data +function reconstructFlattenedVec(flattenedVec) { + const data = flattenedVec.data(); + const lengths = flattenedVec.lengths(); + + let index = 0; + const result = []; + lengths.forEach((length) => { + result.push(data.slice(index, index + length)); + index += length; + }); + return result; +} diff --git a/crates/rust-client/src/store/web_store/mod.rs b/crates/rust-client/src/store/web_store/mod.rs index 0d8d8d5dc..0e8968cbb 100644 --- a/crates/rust-client/src/store/web_store/mod.rs +++ b/crates/rust-client/src/store/web_store/mod.rs @@ -79,12 +79,8 @@ impl Store for WebStore { self.get_sync_height().await } - async fn apply_state_sync_step( - &self, - state_sync_update: StateSyncUpdate, - block_has_relevant_notes: bool, - ) -> Result<(), StoreError> { - self.apply_state_sync_step(state_sync_update, block_has_relevant_notes).await + async fn apply_state_sync(&self, state_sync_update: StateSyncUpdate) -> Result<(), StoreError> { + self.apply_state_sync(state_sync_update).await } // TRANSACTIONS diff --git a/crates/rust-client/src/store/web_store/note/utils.rs b/crates/rust-client/src/store/web_store/note/utils.rs index 0d1c79e0b..4dc4c103f 100644 --- a/crates/rust-client/src/store/web_store/note/utils.rs +++ b/crates/rust-client/src/store/web_store/note/utils.rs @@ -193,17 +193,11 @@ pub fn parse_output_note_idxdb_object( } pub(crate) async fn apply_note_updates_tx(note_updates: &NoteUpdates) -> Result<(), StoreError> { - for input_note in - note_updates.new_input_notes().iter().chain(note_updates.updated_input_notes()) - { + for input_note in note_updates.updated_input_notes() { upsert_input_note_tx(input_note).await?; } - for output_note in note_updates - .new_output_notes() - .iter() - .chain(note_updates.updated_output_notes()) - { + for output_note in note_updates.updated_output_notes() { upsert_output_note_tx(output_note).await?; } diff --git a/crates/rust-client/src/store/web_store/sync/js_bindings.rs b/crates/rust-client/src/store/web_store/sync/js_bindings.rs index 4c8fc7341..31249e78f 100644 --- a/crates/rust-client/src/store/web_store/sync/js_bindings.rs +++ b/crates/rust-client/src/store/web_store/sync/js_bindings.rs @@ -3,6 +3,8 @@ use alloc::{string::String, vec::Vec}; use wasm_bindgen::prelude::*; use wasm_bindgen_futures::*; +use super::flattened_vec::FlattenedU8Vec; + // Sync IndexedDB Operations #[wasm_bindgen(module = "/src/store/web_store/js/sync.js")] @@ -29,9 +31,10 @@ extern "C" { #[wasm_bindgen(js_name = applyStateSync)] pub fn idxdb_apply_state_sync( block_num: String, - block_header: Vec, - chain_mmr_peaks: Vec, - has_client_notes: bool, + flattened_new_block_headers: FlattenedU8Vec, + new_block_nums: Vec, + flattened_chain_mmr_peaks: FlattenedU8Vec, + has_client_notes: Vec, serialized_node_ids: Vec, serialized_nodes: Vec, note_tags_to_remove_as_str: Vec, diff --git a/crates/rust-client/src/store/web_store/sync/mod.rs b/crates/rust-client/src/store/web_store/sync/mod.rs index feba70f27..23a5eaf9c 100644 --- a/crates/rust-client/src/store/web_store/sync/mod.rs +++ b/crates/rust-client/src/store/web_store/sync/mod.rs @@ -29,6 +29,9 @@ use js_bindings::*; mod models; use models::*; +mod flattened_vec; +use flattened_vec::*; + impl WebStore { pub(crate) async fn get_note_tags(&self) -> Result, StoreError> { let promise = idxdb_get_note_tags(); @@ -98,32 +101,39 @@ impl WebStore { Ok(removed_tags) } - pub(super) async fn apply_state_sync_step( + pub(super) async fn apply_state_sync( &self, state_sync_update: StateSyncUpdate, - block_has_relevant_notes: bool, ) -> Result<(), StoreError> { let StateSyncUpdate { - block_header, + block_num, + block_updates, note_updates, transaction_updates, //TODO: Add support for discarded transactions in web store - new_mmr_peaks, - new_authentication_nodes, account_updates, tags_to_remove, } = state_sync_update; // Serialize data for updating state sync and block header - let block_num_as_str = block_header.block_num().to_string(); + let block_num_as_str = block_num.to_string(); // Serialize data for updating block header - let block_header_as_bytes = block_header.to_bytes(); - let new_mmr_peaks_as_bytes = new_mmr_peaks.peaks().to_vec().to_bytes(); + let mut block_headers_as_bytes = vec![]; + let mut new_mmr_peaks_as_bytes = vec![]; + let mut block_nums_as_str = vec![]; + let mut block_has_relevant_notes = vec![]; + + for (block_header, has_client_notes, mmr_peaks) in block_updates.block_headers.iter() { + block_headers_as_bytes.push(block_header.to_bytes()); + new_mmr_peaks_as_bytes.push(mmr_peaks.peaks().to_vec().to_bytes()); + block_nums_as_str.push(block_header.block_num().to_string()); + block_has_relevant_notes.push(*has_client_notes as u8); + } // Serialize data for updating chain MMR nodes let mut serialized_node_ids = Vec::new(); let mut serialized_nodes = Vec::new(); - for (id, node) in new_authentication_nodes.iter() { + for (id, node) in block_updates.new_authentication_nodes.iter() { let serialized_data = serialize_chain_mmr_node(*id, *node)?; serialized_node_ids.push(serialized_data.id); serialized_nodes.push(serialized_data.node); @@ -178,8 +188,9 @@ impl WebStore { let promise = idxdb_apply_state_sync( block_num_as_str, - block_header_as_bytes, - new_mmr_peaks_as_bytes, + flatten_nested_u8_vec(block_headers_as_bytes), + block_nums_as_str, + flatten_nested_u8_vec(new_mmr_peaks_as_bytes), block_has_relevant_notes, serialized_node_ids, serialized_nodes, diff --git a/crates/web-client/package-lock.json b/crates/web-client/package-lock.json index 9c504c5b3..e8b9125bc 100644 --- a/crates/web-client/package-lock.json +++ b/crates/web-client/package-lock.json @@ -1,12 +1,12 @@ { "name": "@demox-labs/miden-sdk", - "version": "0.6.1-next.3", + "version": "0.6.1-next.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@demox-labs/miden-sdk", - "version": "0.6.1-next.3", + "version": "0.6.1-next.4", "dependencies": { "chai-as-promised": "^8.0.0", "dexie": "^4.0.1", From 0d0146a6e68ae8f78a7061c7ed98974df478dbf1 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 14:36:51 -0300 Subject: [PATCH 04/14] chore: update docs --- crates/rust-client/src/note/mod.rs | 16 +++--- crates/rust-client/src/sync/block_header.rs | 6 +++ crates/rust-client/src/sync/state_sync.rs | 58 +++++++++++---------- 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 992f626b1..2a2fd4947 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -248,9 +248,9 @@ pub async fn get_input_note_with_id_prefix( /// Contains note changes to apply to the store. #[derive(Clone, Debug, Default)] pub struct NoteUpdates { - /// A map of updated input note records corresponding to locally-tracked input notes. + /// A map of new and updated input note records to be upserted in the store. updated_input_notes: BTreeMap, - /// A map of updated output note records corresponding to locally-tracked output notes. + /// A map of updated output note records to be upserted in the store. updated_output_notes: BTreeMap, } @@ -272,14 +272,12 @@ impl NoteUpdates { } } - /// Returns all updated input note records. That is, any input notes that are locally tracked - /// and have been updated. + /// Returns all input note records that have been updated. pub fn updated_input_notes(&self) -> impl Iterator { self.updated_input_notes.values() } - /// Returns all updated output note records. That is, any output notes that are locally tracked - /// and have been updated. + /// Returns all updated output note records that have been updated. pub fn updated_output_notes(&self) -> impl Iterator { self.updated_output_notes.values() } @@ -339,14 +337,18 @@ impl NoteUpdates { } } + /// Returns a mutable reference to the input note record with the provided ID if it exists. pub fn get_input_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut InputNoteRecord> { self.updated_input_notes.get_mut(note_id) } + /// Returns a mutable reference to the output note record with the provided ID if it exists. pub fn get_output_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut OutputNoteRecord> { self.updated_output_notes.get_mut(note_id) } + /// Returns a mutable reference to the input note record with the provided nullifier if it + /// exists. pub fn get_input_note_by_nullifier( &mut self, nullifier: Nullifier, @@ -354,6 +356,8 @@ impl NoteUpdates { self.updated_input_notes.values_mut().find(|note| note.nullifier() == nullifier) } + /// Returns a mutable reference to the output note record with the provided nullifier if it + /// exists. pub fn get_output_note_by_nullifier( &mut self, nullifier: Nullifier, diff --git a/crates/rust-client/src/sync/block_header.rs b/crates/rust-client/src/sync/block_header.rs index a60cd9f22..76fe9940b 100644 --- a/crates/rust-client/src/sync/block_header.rs +++ b/crates/rust-client/src/sync/block_header.rs @@ -13,9 +13,15 @@ use crate::{ Client, ClientError, }; +/// Contains all the block information that needs to be added in the client's store after a sync. + #[derive(Debug, Clone, Default)] pub struct BlockUpdates { + /// New block headers to be stored, along with a flag indicating whether the block contains + /// notes that are relevant to the client and the MMR peaks for the block. pub block_headers: Vec<(BlockHeader, bool, MmrPeaks)>, + /// New authentication nodes that are meant to be stored in order to authenticate block + /// headers. pub new_authentication_nodes: Vec<(InOrderIndex, Digest)>, } diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 82bc9812e..61c27ced2 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -29,15 +29,15 @@ use crate::{ #[derive(Default)] /// Contains all information needed to apply the update in the store after syncing with the node. pub struct StateSyncUpdate { + /// The block number of the last block that was synced. pub block_num: BlockNumber, - /// The new block header, returned as part of the - /// [StateSyncInfo](crate::rpc::domain::sync::StateSyncInfo) + /// New blocks and authentication nodes. pub block_updates: BlockUpdates, - /// Information about note changes after the sync. + /// New and updated notes to be upserted in the store. pub note_updates: NoteUpdates, - /// Information about transaction changes after the sync. + /// Committed and discarded transactions after the sync. pub transaction_updates: TransactionUpdates, - /// Information abount account changes after the sync. + /// Public account updates and mismatched private accounts after the sync. pub account_updates: AccountUpdates, /// Tag records that are no longer relevant. pub tags_to_remove: Vec, @@ -138,7 +138,10 @@ pub struct StateSync { on_transaction_committed: OnTransactionCommitted, /// Callback to be executed when a nullifier is received. on_nullifier_received: OnNullifierReceived, + /// Callback to be executed when a block header is received. on_block_received: OnBlockHeaderReceived, + /// The state sync update that will be returned after the sync process is completed. It + /// agregates all the updates that come from each sync step. state_sync_update: StateSyncUpdate, } @@ -151,6 +154,7 @@ impl StateSync { /// * `on_note_received` - A callback to be executed when a new note inclusion is received. /// * `on_committed_transaction` - A callback to be executed when a transaction is committed. /// * `on_nullifier_received` - A callback to be executed when a nullifier is received. + /// * `on_block_received` - A callback to be executed when a block header is received. pub fn new( rpc_api: Arc, on_note_received: OnNoteReceived, @@ -168,25 +172,15 @@ impl StateSync { } } - /// Executes a single step of the state sync process, returning the changes that should be - /// applied to the store. + /// Executes a single step of the state sync process, returning `true` if the client should + /// continue syncing and `false` if the client has reached the chain tip. /// /// A step in this context means a single request to the node to get the next relevant block and /// the changes that happened in it. This block may not be the last one in the chain and /// the client may need to call this method multiple times until it reaches the chain tip. - /// Wheter or not the client has reached the chain tip is indicated by the returned - /// [SyncStatus] variant. `None` is returned if the client is already synced with the chain tip - /// and there are no new changes. /// - /// # Arguments - /// * `current_block` - The latest tracked block header. - /// * `current_block_has_relevant_notes` - A flag indicating if the current block has notes that - /// are relevant to the client. This is used to determine whether new MMR authentication nodes - /// are stored for this block. - /// * `current_partial_mmr` - The current partial MMR. - /// * `accounts` - The headers of tracked accounts. - /// * `note_tags` - The note tags to be used in the sync state request. - /// * `unspent_nullifiers` - The nullifiers of tracked notes that haven't been consumed. + /// The `sync_state_update` field of the struct will be updated with the new changes from this + /// step. async fn sync_state_step( &mut self, current_block: BlockHeader, @@ -248,6 +242,18 @@ impl StateSync { } } + /// Syncs the state of the client with the chain tip of the node, returning the updates that + /// should be applied to the store. + /// + /// # Arguments + /// * `current_block` - The latest tracked block header. + /// * `current_block_has_relevant_notes` - A flag indicating if the current block has notes that + /// are relevant to the client. This is used to determine whether new MMR authentication nodes + /// are stored for this block. + /// * `current_partial_mmr` - The current partial MMR. + /// * `accounts` - The headers of tracked accounts. + /// * `note_tags` - The note tags to be used in the sync state request. + /// * `unspent_nullifiers` - The nullifiers of tracked notes that haven't been consumed. pub async fn sync_state( mut self, mut current_block: BlockHeader, @@ -285,7 +291,8 @@ impl StateSync { // HELPERS // -------------------------------------------------------------------------------------------- - /// Compares the state of tracked accounts with the updates received from the node and returns + /// Compares the state of tracked accounts with the updates received from the node and updates the + /// `state_sync_update` with the details of /// the accounts that need to be updated. /// /// When a mismatch is detected, two scenarios are possible: @@ -347,7 +354,8 @@ impl StateSync { } /// Applies the changes received from the sync response to the notes and transactions tracked - /// by the client. It returns the updates that should be applied to the store. + /// by the client and updates the + /// `state_sync_update` accordingly. /// /// This method uses the callbacks provided to the [StateSync] component to apply the changes. /// @@ -536,8 +544,6 @@ pub async fn on_transaction_committed( mut note_updates: NoteUpdates, transaction_update: TransactionUpdate, ) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { - // TODO: This could be improved if we add a filter to get only notes that are being processed by - // a specific transaction let processing_notes = store.get_input_notes(NoteFilter::Processing).await?; let consumed_input_notes: Vec = processing_notes .into_iter() @@ -668,14 +674,12 @@ pub(crate) async fn check_block_relevance( .inclusion_proof() .is_some_and(|proof| proof.location().block_num() != new_block_number) { - // This note wasn't received in the current block, so it's not relevant + // This note wasn't received in the current block, so it shouldn't be considered continue; } - // TODO: Map the below error into a better representation (ie, we expected to be able - // to convert here) if !note_screener - .check_relevance(&input_note.try_into().map_err(ClientError::NoteRecordError)?) + .check_relevance(&input_note.try_into().expect("Committed notes should have metadata")) .await? .is_empty() { From 21829d20a9a39cd6ff8984b934082a42922d0b25 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 15:07:10 -0300 Subject: [PATCH 05/14] add failing test related to TODOs --- tests/integration/onchain_tests.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/onchain_tests.rs b/tests/integration/onchain_tests.rs index 2761bc3a3..ffe3aba30 100644 --- a/tests/integration/onchain_tests.rs +++ b/tests/integration/onchain_tests.rs @@ -84,8 +84,13 @@ async fn test_onchain_notes_flow() { execute_tx_and_sync(&mut client_2, basic_wallet_1.id(), tx_request).await; // sync client 3 (basic account 2) + client_3.add_note_tag(note.metadata().tag()).await.unwrap(); client_3.sync_state().await.unwrap(); - // client 3 should only have one note + + // client 3 should have two notes, the one directed to them and the one consumed by client 2 (which should come from the tag added) + assert_eq!(client_3.get_input_notes(NoteFilter::Committed).await.unwrap().len(), 1); + assert_eq!(client_3.get_input_notes(NoteFilter::Consumed).await.unwrap().len(), 1); + let note = client_3 .get_input_notes(NoteFilter::Committed) .await From f168a84334e925044ccad1449ebe7a33dd8387b1 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 15:07:32 -0300 Subject: [PATCH 06/14] fix: add nullifiers for new untracked notes in each step --- crates/rust-client/src/sync/state_sync.rs | 24 ++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 61c27ced2..cc9047809 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -261,23 +261,37 @@ impl StateSync { mut current_partial_mmr: PartialMmr, accounts: Vec, note_tags: Vec, - unspent_nullifiers: Vec, + mut unspent_nullifiers: Vec, ) -> Result { loop { + // New nullfiers should be added for new untracked notes that were added in previous steps + unspent_nullifiers.append( + &mut self + .state_sync_update + .note_updates + .updated_input_notes() + .filter(|note| { + note.is_committed() && !unspent_nullifiers.contains(¬e.nullifier()) + }) + .map(|note| note.nullifier()) + .collect::>(), + ); + if !self .sync_state_step( current_block, current_block_has_relevant_notes, &mut current_partial_mmr, &accounts, - ¬e_tags, //TODO: get note tags from notes in the updates - &unspent_nullifiers, //TODO: get nullifiers from notes in the updates + ¬e_tags, + &unspent_nullifiers, ) .await? { return Ok(self.state_sync_update); } + // Update the current block for the next step (current_block, current_block_has_relevant_notes, ..) = self .state_sync_update .block_updates @@ -291,8 +305,8 @@ impl StateSync { // HELPERS // -------------------------------------------------------------------------------------------- - /// Compares the state of tracked accounts with the updates received from the node and updates the - /// `state_sync_update` with the details of + /// Compares the state of tracked accounts with the updates received from the node and updates + /// the `state_sync_update` with the details of /// the accounts that need to be updated. /// /// When a mismatch is detected, two scenarios are possible: From 174c9954b7bcd8dc05f0ee06b0206d3ed01f7d01 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 15:59:22 -0300 Subject: [PATCH 07/14] use `Arc>` in callbacks --- crates/rust-client/src/sync/state_sync.rs | 137 ++++++++++++---------- tests/integration/onchain_tests.rs | 3 +- 2 files changed, 75 insertions(+), 65 deletions(-) diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index cc9047809..1c299d46a 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -8,6 +8,7 @@ use miden_objects::{ note::{NoteId, NoteInclusionProof, NoteTag, Nullifier}, Digest, }; +use miden_tx::utils::sync::RwLock; use tracing::info; use super::{block_header::BlockUpdates, get_nullifier_prefix, NoteTagRecord, SyncSummary}; @@ -80,10 +81,10 @@ impl From<&StateSyncUpdate> for SyncSummary { /// that should be queried from the node and start being tracked. pub type OnNoteReceived = Box< dyn Fn( - NoteUpdates, + Arc>, CommittedNote, BlockHeader, - ) -> Pin), ClientError>>>>, + ) -> Pin, ClientError>>>>, >; /// Callback to be executed when a transaction is marked committed in the sync response. It receives @@ -91,10 +92,9 @@ pub type OnNoteReceived = Box< /// updates that should be applied to the store as a result of the transaction being committed. pub type OnTransactionCommitted = Box< dyn Fn( - NoteUpdates, + Arc>, TransactionUpdate, - ) - -> Pin>>>, + ) -> Pin>>>, >; /// Callback to be executed when a nullifier is received in the sync response. If a note was @@ -104,10 +104,9 @@ pub type OnTransactionCommitted = Box< /// store as a result of the nullifier being received. pub type OnNullifierReceived = Box< dyn Fn( - NoteUpdates, + Arc>, NullifierUpdate, - ) - -> Pin>>>, + ) -> Pin>>>, >; pub type OnBlockHeaderReceived = Box< @@ -263,8 +262,19 @@ impl StateSync { note_tags: Vec, mut unspent_nullifiers: Vec, ) -> Result { - loop { - // New nullfiers should be added for new untracked notes that were added in previous steps + while self + .sync_state_step( + current_block, + current_block_has_relevant_notes, + &mut current_partial_mmr, + &accounts, + ¬e_tags, + &unspent_nullifiers, + ) + .await? + { + // New nullfiers should be added for new untracked notes that were added in previous + // steps unspent_nullifiers.append( &mut self .state_sync_update @@ -277,20 +287,6 @@ impl StateSync { .collect::>(), ); - if !self - .sync_state_step( - current_block, - current_block_has_relevant_notes, - &mut current_partial_mmr, - &accounts, - ¬e_tags, - &unspent_nullifiers, - ) - .await? - { - return Ok(self.state_sync_update); - } - // Update the current block for the next step (current_block, current_block_has_relevant_notes, ..) = self .state_sync_update @@ -300,6 +296,23 @@ impl StateSync { .cloned() .expect("At least one block header should be present"); } + + // We can remove tags from notes that got committed + let tags_to_remove: Vec = self + .state_sync_update + .note_updates + .committed_input_notes() + .map(|note| { + NoteTagRecord::with_note_source( + note.metadata().expect("Committed note should have metadata").tag(), + note.id(), + ) + }) + .collect(); + + self.state_sync_update.tags_to_remove.extend(tags_to_remove); + + Ok(self.state_sync_update) } // HELPERS @@ -392,58 +405,48 @@ impl StateSync { block_header: BlockHeader, ) -> Result<(), ClientError> { let mut public_note_ids = vec![]; - let mut note_updates = self.state_sync_update.note_updates.clone(); + let note_updates = Arc::new(RwLock::new(self.state_sync_update.note_updates.clone())); // TODO: look to remove this clone + // Process note inclusions for committed_note in note_inclusions { - let (new_note_updates, new_note_ids) = - (self.on_note_received)(note_updates, committed_note, block_header).await?; - note_updates = new_note_updates; + let new_note_ids = + (self.on_note_received)(note_updates.clone(), committed_note, block_header).await?; public_note_ids.extend(new_note_ids); } - let new_public_notes = - self.fetch_public_note_details(&public_note_ids, &block_header).await?; - - note_updates.insert_or_ignore_notes(&new_public_notes, &vec![]); - - // We can remove tags from notes that got committed - let tags_to_remove: Vec = note_updates - .updated_input_notes() - .filter(|note| note.is_committed()) - .map(|note| { - NoteTagRecord::with_note_source( - note.metadata().expect("Committed note should have metadata").tag(), - note.id(), - ) - }) - .collect(); - - self.state_sync_update.tags_to_remove.extend(tags_to_remove); - + // Process committed transactions for transaction_update in transactions { - let (new_note_updates, new_transaction_update) = - (self.on_transaction_committed)(note_updates, transaction_update).await?; + let new_transaction_update = + (self.on_transaction_committed)(note_updates.clone(), transaction_update).await?; // Remove nullifiers if they were consumed by the transaction nullifiers.retain(|nullifier| { - !new_note_updates + !note_updates + .read() .updated_input_notes() .any(|note| note.nullifier() == nullifier.nullifier) }); - note_updates = new_note_updates; self.state_sync_update.transaction_updates.extend(new_transaction_update); } + // Process nullifiers for nullifier_update in nullifiers { - let (new_note_updates, new_transaction_update) = - (self.on_nullifier_received)(note_updates, nullifier_update).await?; + let new_transaction_update = + (self.on_nullifier_received)(note_updates.clone(), nullifier_update).await?; - note_updates = new_note_updates; self.state_sync_update.transaction_updates.extend(new_transaction_update); } - self.state_sync_update.note_updates = note_updates; + self.state_sync_update.note_updates = note_updates.read().clone(); // TODO: look to remove this clone + + // Process new untracked notes + let new_public_notes = + self.fetch_public_note_details(&public_note_ids, &block_header).await?; + + self.state_sync_update + .note_updates + .insert_or_ignore_notes(&new_public_notes, &vec![]); Ok(()) } @@ -506,12 +509,14 @@ async fn apply_mmr_changes( /// Default implementation of the [OnNoteReceived] callback. It queries the store for the committed /// note and updates the note records accordingly. If the note is not being tracked, it returns the /// note ID to be queried from the node so it can be queried from the node and tracked. +#[allow(clippy::await_holding_lock)] pub async fn on_note_received( store: Arc, - mut note_updates: NoteUpdates, + note_updates: Arc>, committed_note: CommittedNote, block_header: BlockHeader, -) -> Result<(NoteUpdates, Vec), ClientError> { +) -> Result, ClientError> { + let mut note_updates = note_updates.write(); let inclusion_proof = NoteInclusionProof::new( block_header.block_num(), committed_note.note_index(), @@ -547,17 +552,19 @@ pub async fn on_note_received( new_note_ids.push(*committed_note.note_id()); } - Ok((note_updates, new_note_ids)) + Ok(new_note_ids) } /// Default implementation of the [OnTransactionCommitted] callback. It queries the store for the /// input notes that were consumed by the transaction and updates the note records accordingly. It /// also returns the committed transaction update to be applied to the store. +#[allow(clippy::await_holding_lock)] pub async fn on_transaction_committed( store: Arc, - mut note_updates: NoteUpdates, + note_updates: Arc>, transaction_update: TransactionUpdate, -) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { +) -> Result { + let mut note_updates = note_updates.write(); let processing_notes = store.get_input_notes(NoteFilter::Processing).await?; let consumed_input_notes: Vec = processing_notes .into_iter() @@ -594,17 +601,19 @@ pub async fn on_transaction_committed( output_note_record.nullifier_received(nullifier, transaction_update.block_num)?; } - Ok((note_updates, TransactionUpdates::new(vec![transaction_update], vec![]))) + Ok(TransactionUpdates::new(vec![transaction_update], vec![])) } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes /// that match the nullifier and updates the note records accordingly. It also returns the /// transactions that should be discarded as they weren't committed when the nullifier was received. +#[allow(clippy::await_holding_lock)] pub async fn on_nullifier_received( store: Arc, - mut note_updates: NoteUpdates, + note_updates: Arc>, nullifier_update: NullifierUpdate, -) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { +) -> Result { + let mut note_updates = note_updates.write(); let mut discarded_transactions = vec![]; note_updates.insert_or_ignore_notes( @@ -638,7 +647,7 @@ pub async fn on_nullifier_received( .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)?; } - Ok((note_updates, TransactionUpdates::new(vec![], discarded_transactions))) + Ok(TransactionUpdates::new(vec![], discarded_transactions)) } pub async fn on_block_received( diff --git a/tests/integration/onchain_tests.rs b/tests/integration/onchain_tests.rs index ffe3aba30..3b29f8b28 100644 --- a/tests/integration/onchain_tests.rs +++ b/tests/integration/onchain_tests.rs @@ -87,7 +87,8 @@ async fn test_onchain_notes_flow() { client_3.add_note_tag(note.metadata().tag()).await.unwrap(); client_3.sync_state().await.unwrap(); - // client 3 should have two notes, the one directed to them and the one consumed by client 2 (which should come from the tag added) + // client 3 should have two notes, the one directed to them and the one consumed by client 2 + // (which should come from the tag added) assert_eq!(client_3.get_input_notes(NoteFilter::Committed).await.unwrap().len(), 1); assert_eq!(client_3.get_input_notes(NoteFilter::Consumed).await.unwrap().len(), 1); From c1fe5fa6f77f38ab3e1259d0e53006372b7138c8 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Thu, 30 Jan 2025 18:53:49 -0300 Subject: [PATCH 08/14] feat: reduce the number of callbacks --- crates/rust-client/src/note/mod.rs | 4 +- crates/rust-client/src/sync/mod.rs | 39 +-- crates/rust-client/src/sync/state_sync.rs | 299 +++++++--------------- 3 files changed, 96 insertions(+), 246 deletions(-) diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 2a2fd4947..41726b2df 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -325,8 +325,8 @@ impl NoteUpdates { pub fn insert_or_ignore_notes( &mut self, - input_notes: &Vec, - output_notes: &Vec, + input_notes: &[InputNoteRecord], + output_notes: &[OutputNoteRecord], ) { for note in input_notes { self.updated_input_notes.entry(note.id()).or_insert(note.clone()); diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index f5d42b65b..f01da00e6 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -74,8 +74,8 @@ pub use tag::{NoteTagRecord, NoteTagSource}; mod state_sync; pub use state_sync::{ - on_block_received, on_note_received, on_nullifier_received, on_transaction_committed, - OnNoteReceived, OnNullifierReceived, OnTransactionCommitted, StateSync, StateSyncUpdate, + on_note_received, on_nullifier_received, OnNoteReceived, OnNullifierReceived, StateSync, + StateSyncUpdate, }; /// Contains stats about the sync operation. @@ -182,51 +182,24 @@ impl Client { self.rpc_api.clone(), Box::new({ let store_clone = self.store.clone(); - move |note_updates, committed_note, block_header| { + move |note_updates, committed_note, block_header, new_public_notes| { Box::pin(on_note_received( store_clone.clone(), note_updates, committed_note, block_header, + new_public_notes, )) } }), Box::new({ let store_clone = self.store.clone(); - move |note_updates, transaction_update| { - Box::pin(on_transaction_committed( - store_clone.clone(), - note_updates, - transaction_update, - )) - } - }), - Box::new({ - let store_clone = self.store.clone(); - move |note_updates, nullifier_update| { + move |note_updates, nullifier_update, committed_transactions| { Box::pin(on_nullifier_received( store_clone.clone(), note_updates, nullifier_update, - )) - } - }), - Box::new({ - let store_clone = self.store.clone(); - move |new_block_header, - note_updates, - current_block_header, - current_block_has_relevant_notes, - current_partial_mmr, - mmr_delta| { - Box::pin(on_block_received( - store_clone.clone(), - new_block_header, - note_updates, - current_block_header, - current_block_has_relevant_notes, - current_partial_mmr, - mmr_delta, + committed_transactions, )) } }), diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 1c299d46a..93938f299 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -75,51 +75,25 @@ impl From<&StateSyncUpdate> for SyncSummary { // SYNC CALLBACKS // ================================================================================================ -/// Callback to be executed when a new note inclusion is received in the sync response. It receives -/// the committed note received from the node and the block header in which the note was included. -/// It returns the note updates that should be applied to the store and a list of public note IDs -/// that should be queried from the node and start being tracked. +/// TODO: document pub type OnNoteReceived = Box< dyn Fn( Arc>, CommittedNote, BlockHeader, - ) -> Pin, ClientError>>>>, + Arc>, + ) -> Pin>>>, >; -/// Callback to be executed when a transaction is marked committed in the sync response. It receives -/// the transaction update received from the node. It returns the note updates and transaction -/// updates that should be applied to the store as a result of the transaction being committed. -pub type OnTransactionCommitted = Box< - dyn Fn( - Arc>, - TransactionUpdate, - ) -> Pin>>>, ->; - -/// Callback to be executed when a nullifier is received in the sync response. If a note was -/// consumed by a committed transaction provided in the [OnTransactionCommitted] callback, its -/// nullifier will not be passed to this callback. It receives the nullifier update received from -/// the node. It returns the note updates and transaction updates that should be applied to the -/// store as a result of the nullifier being received. +/// TODO: document pub type OnNullifierReceived = Box< dyn Fn( Arc>, NullifierUpdate, + Arc>, ) -> Pin>>>, >; -pub type OnBlockHeaderReceived = Box< - dyn Fn( - BlockHeader, - NoteUpdates, - BlockHeader, - bool, - PartialMmr, - MmrDelta, - ) -> Pin>>>, ->; - // STATE SYNC // ================================================================================================ @@ -133,12 +107,8 @@ pub struct StateSync { rpc_api: Arc, /// Callback to be executed when a new note inclusion is received. on_note_received: OnNoteReceived, - /// Callback to be executed when a transaction is committed. - on_transaction_committed: OnTransactionCommitted, /// Callback to be executed when a nullifier is received. on_nullifier_received: OnNullifierReceived, - /// Callback to be executed when a block header is received. - on_block_received: OnBlockHeaderReceived, /// The state sync update that will be returned after the sync process is completed. It /// agregates all the updates that come from each sync step. state_sync_update: StateSyncUpdate, @@ -153,20 +123,15 @@ impl StateSync { /// * `on_note_received` - A callback to be executed when a new note inclusion is received. /// * `on_committed_transaction` - A callback to be executed when a transaction is committed. /// * `on_nullifier_received` - A callback to be executed when a nullifier is received. - /// * `on_block_received` - A callback to be executed when a block header is received. pub fn new( rpc_api: Arc, on_note_received: OnNoteReceived, - on_transaction_committed: OnTransactionCommitted, on_nullifier_received: OnNullifierReceived, - on_block_received: OnBlockHeaderReceived, ) -> Self { Self { rpc_api, on_note_received, - on_transaction_committed, on_nullifier_received, - on_block_received, state_sync_update: StateSyncUpdate::default(), } } @@ -213,26 +178,27 @@ impl StateSync { self.account_state_sync(accounts, &response.account_hash_updates).await?; - self.note_state_sync( - response.note_inclusions, - response.transactions, - response.nullifiers, - response.block_header, - ) - .await?; + let found_relevant_note = self + .note_state_sync( + response.note_inclusions, + response.transactions, + response.nullifiers, + response.block_header, + ) + .await?; - let (new_block_updates, new_partial_mmr) = (self.on_block_received)( - response.block_header, - self.state_sync_update.note_updates.clone(), + let (new_mmr_peaks, new_authentication_nodes) = apply_mmr_changes( current_block, current_block_has_relevant_notes, - current_partial_mmr.clone(), + current_partial_mmr, response.mmr_delta, ) .await?; - self.state_sync_update.block_updates.extend(new_block_updates); - *current_partial_mmr = new_partial_mmr; + self.state_sync_update.block_updates.extend(BlockUpdates { + block_headers: vec![(response.block_header, found_relevant_note, new_mmr_peaks)], + new_authentication_nodes, + }); if response.chain_tip == response.block_header.block_num() { Ok(false) @@ -401,54 +367,51 @@ impl StateSync { &mut self, note_inclusions: Vec, transactions: Vec, - mut nullifiers: Vec, + nullifiers: Vec, block_header: BlockHeader, - ) -> Result<(), ClientError> { - let mut public_note_ids = vec![]; + ) -> Result { + let public_note_ids: Vec = note_inclusions + .iter() + .filter_map(|note| (!note.metadata().is_private()).then_some(*note.note_id())) + .collect(); + + let mut found_relevant_note = false; let note_updates = Arc::new(RwLock::new(self.state_sync_update.note_updates.clone())); // TODO: look to remove this clone // Process note inclusions + let new_public_notes = + Arc::new(self.fetch_public_note_details(&public_note_ids, &block_header).await?); for committed_note in note_inclusions { - let new_note_ids = - (self.on_note_received)(note_updates.clone(), committed_note, block_header).await?; - public_note_ids.extend(new_note_ids); - } - - // Process committed transactions - for transaction_update in transactions { - let new_transaction_update = - (self.on_transaction_committed)(note_updates.clone(), transaction_update).await?; - - // Remove nullifiers if they were consumed by the transaction - nullifiers.retain(|nullifier| { - !note_updates - .read() - .updated_input_notes() - .any(|note| note.nullifier() == nullifier.nullifier) - }); - - self.state_sync_update.transaction_updates.extend(new_transaction_update); + let note_is_relevant = (self.on_note_received)( + note_updates.clone(), + committed_note, + block_header, + new_public_notes.clone(), + ) + .await?; + found_relevant_note |= note_is_relevant; } // Process nullifiers + let committed_transactions = Arc::new(transactions.clone()); for nullifier_update in nullifiers { - let new_transaction_update = - (self.on_nullifier_received)(note_updates.clone(), nullifier_update).await?; + let new_transaction_update = (self.on_nullifier_received)( + note_updates.clone(), + nullifier_update, + committed_transactions.clone(), + ) + .await?; self.state_sync_update.transaction_updates.extend(new_transaction_update); } - self.state_sync_update.note_updates = note_updates.read().clone(); // TODO: look to remove this clone - - // Process new untracked notes - let new_public_notes = - self.fetch_public_note_details(&public_note_ids, &block_header).await?; - self.state_sync_update - .note_updates - .insert_or_ignore_notes(&new_public_notes, &vec![]); + .transaction_updates + .extend(TransactionUpdates::new(transactions, vec![])); - Ok(()) + self.state_sync_update.note_updates = note_updates.read().clone(); // TODO: look to remove this clone + + Ok(found_relevant_note) } /// Queries the node for all received notes that aren't being locally tracked in the client. @@ -515,7 +478,8 @@ pub async fn on_note_received( note_updates: Arc>, committed_note: CommittedNote, block_header: BlockHeader, -) -> Result, ClientError> { + new_public_notes: Arc>, +) -> Result { let mut note_updates = note_updates.write(); let inclusion_proof = NoteInclusionProof::new( block_header.block_num(), @@ -524,7 +488,7 @@ pub async fn on_note_received( )?; let mut is_tracked_note = false; - let mut new_note_ids = vec![]; + let mut block_is_relevant = false; note_updates.insert_or_ignore_notes( &store.get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])).await?, @@ -536,6 +500,7 @@ pub async fn on_note_received( if let Some(input_note_record) = note_updates.get_input_note_by_id(committed_note.note_id()) { // The note belongs to our locally tracked set of input notes is_tracked_note = true; + block_is_relevant = true; //TODO: Check if this is always true input_note_record .inclusion_proof_received(inclusion_proof.clone(), committed_note.metadata())?; input_note_record.block_header_received(block_header)?; @@ -548,60 +513,24 @@ pub async fn on_note_received( } if !is_tracked_note { - // The note is public and we are not tracking it, push to the list of IDs to query - new_note_ids.push(*committed_note.note_id()); - } - - Ok(new_note_ids) -} - -/// Default implementation of the [OnTransactionCommitted] callback. It queries the store for the -/// input notes that were consumed by the transaction and updates the note records accordingly. It -/// also returns the committed transaction update to be applied to the store. -#[allow(clippy::await_holding_lock)] -pub async fn on_transaction_committed( - store: Arc, - note_updates: Arc>, - transaction_update: TransactionUpdate, -) -> Result { - let mut note_updates = note_updates.write(); - let processing_notes = store.get_input_notes(NoteFilter::Processing).await?; - let consumed_input_notes: Vec = processing_notes - .into_iter() - .filter(|note_record| { - note_record.consumer_transaction_id() == Some(&transaction_update.transaction_id) - }) - .collect(); - - let consumed_output_notes = store - .get_output_notes(NoteFilter::Nullifiers( - consumed_input_notes.iter().map(|n| n.nullifier()).collect(), - )) - .await?; - - note_updates.insert_or_ignore_notes(&consumed_input_notes, &consumed_output_notes); - - for store_note in consumed_input_notes { - let input_note_record = note_updates - .get_input_note_by_id(&store_note.id()) - .expect("Input note should be present in the note updates after being inserted"); - - input_note_record.transaction_committed( - transaction_update.transaction_id, - transaction_update.block_num, - )?; - } + // The note wasn't being tracked but it came in the sync response, it means it matched a + // note tag we are tracking and it needs to be inserted in the store + if let Some(public_note) = + new_public_notes.iter().find(|note| ¬e.id() == committed_note.note_id()) + { + note_updates.insert_or_ignore_notes(&[public_note.clone()], &[]); - for store_note in consumed_output_notes { - // SAFETY: Output notes were queried from a nullifier list and should have a nullifier - let nullifier = store_note.nullifier().unwrap(); - let output_note_record = note_updates - .get_output_note_by_id(&store_note.id()) - .expect("Output note should be present in the note updates after being inserted"); - output_note_record.nullifier_received(nullifier, transaction_update.block_num)?; + // If the note isn't consumable by the client then the block isn't relevant + block_is_relevant = !NoteScreener::new(store) + .check_relevance( + &public_note.try_into().expect("Committed notes should have metadata"), + ) + .await? + .is_empty(); + } } - Ok(TransactionUpdates::new(vec![transaction_update], vec![])) + Ok(block_is_relevant) } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes @@ -612,6 +541,7 @@ pub async fn on_nullifier_received( store: Arc, note_updates: Arc>, nullifier_update: NullifierUpdate, + transaction_updates: Arc>, ) -> Result { let mut note_updates = note_updates.write(); let mut discarded_transactions = vec![]; @@ -628,16 +558,26 @@ pub async fn on_nullifier_received( if let Some(input_note_record) = note_updates.get_input_note_by_nullifier(nullifier_update.nullifier) { - if input_note_record.is_processing() { - discarded_transactions.push( - *input_note_record - .consumer_transaction_id() - .expect("Processing note should have consumer transaction id"), - ); + if let Some(consumer_transaction) = transaction_updates.iter().find(|t| { + input_note_record + .consumer_transaction_id() + .map_or(false, |id| id == &t.transaction_id) + }) { + // The note was being processed by a local transaction that just got committed + input_note_record.transaction_committed( + consumer_transaction.transaction_id, + consumer_transaction.block_num, + )?; + } else { + // The note was consumed by an external transaction + if let Some(id) = input_note_record.consumer_transaction_id() { + // The note was being processed by a local transaction that didn't end up being + // committed so it should be discarded + discarded_transactions.push(*id); + } + input_note_record + .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } - - input_note_record - .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } if let Some(output_note_record) = @@ -649,66 +589,3 @@ pub async fn on_nullifier_received( Ok(TransactionUpdates::new(vec![], discarded_transactions)) } - -pub async fn on_block_received( - store: Arc, - new_block_header: BlockHeader, - note_updates: NoteUpdates, - current_block_header: BlockHeader, - current_block_has_relevant_notes: bool, - mut current_partial_mmr: PartialMmr, - mmr_delta: MmrDelta, -) -> Result<(BlockUpdates, PartialMmr), ClientError> { - let (mmr_peaks, new_authentication_nodes) = apply_mmr_changes( - current_block_header, - current_block_has_relevant_notes, - &mut current_partial_mmr, - mmr_delta, - ) - .await?; - - let block_relevance = - check_block_relevance(store.clone(), new_block_header.block_num(), note_updates).await?; - - Ok(( - BlockUpdates { - block_headers: vec![(new_block_header, block_relevance, mmr_peaks)], - new_authentication_nodes, - }, - current_partial_mmr, - )) -} - -/// Checks the relevance of the block by verifying if any of the input notes in the block are -/// relevant to the client. If any of the notes are relevant, the function returns `true`. -pub(crate) async fn check_block_relevance( - store: Arc, - new_block_number: BlockNumber, - note_updates: NoteUpdates, -) -> Result { - // We'll only do the check for either incoming public notes or expected input notes as - // output notes are not really candidates to be consumed here. - - let note_screener = NoteScreener::new(store); - - // Find all relevant Input Notes using the note checker - for input_note in note_updates.committed_input_notes() { - if input_note - .inclusion_proof() - .is_some_and(|proof| proof.location().block_num() != new_block_number) - { - // This note wasn't received in the current block, so it shouldn't be considered - continue; - } - - if !note_screener - .check_relevance(&input_note.try_into().expect("Committed notes should have metadata")) - .await? - .is_empty() - { - return Ok(true); - } - } - - Ok(false) -} From 109fcc64a390d65fce5b12306051cb778ab686a5 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Fri, 31 Jan 2025 17:46:20 -0300 Subject: [PATCH 09/14] remove `NoteUpdates` as callback parameter --- crates/rust-client/src/note/mod.rs | 36 +------ crates/rust-client/src/sync/mod.rs | 6 +- crates/rust-client/src/sync/state_sync.rs | 121 ++++++++++++---------- 3 files changed, 70 insertions(+), 93 deletions(-) diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 41726b2df..518766729 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -323,28 +323,9 @@ impl NoteUpdates { BTreeSet::from_iter(consumed_input_note_ids.chain(consumed_output_note_ids)) } - pub fn insert_or_ignore_notes( - &mut self, - input_notes: &[InputNoteRecord], - output_notes: &[OutputNoteRecord], - ) { - for note in input_notes { - self.updated_input_notes.entry(note.id()).or_insert(note.clone()); - } - - for note in output_notes { - self.updated_output_notes.entry(note.id()).or_insert(note.clone()); - } - } - - /// Returns a mutable reference to the input note record with the provided ID if it exists. - pub fn get_input_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut InputNoteRecord> { - self.updated_input_notes.get_mut(note_id) - } - - /// Returns a mutable reference to the output note record with the provided ID if it exists. - pub fn get_output_note_by_id(&mut self, note_id: &NoteId) -> Option<&mut OutputNoteRecord> { - self.updated_output_notes.get_mut(note_id) + pub fn extend(&mut self, other: NoteUpdates) { + self.updated_input_notes.extend(other.updated_input_notes); + self.updated_output_notes.extend(other.updated_output_notes); } /// Returns a mutable reference to the input note record with the provided nullifier if it @@ -355,15 +336,4 @@ impl NoteUpdates { ) -> Option<&mut InputNoteRecord> { self.updated_input_notes.values_mut().find(|note| note.nullifier() == nullifier) } - - /// Returns a mutable reference to the output note record with the provided nullifier if it - /// exists. - pub fn get_output_note_by_nullifier( - &mut self, - nullifier: Nullifier, - ) -> Option<&mut OutputNoteRecord> { - self.updated_output_notes - .values_mut() - .find(|note| note.nullifier() == Some(nullifier)) - } } diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index f01da00e6..3321a3e8c 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -182,10 +182,9 @@ impl Client { self.rpc_api.clone(), Box::new({ let store_clone = self.store.clone(); - move |note_updates, committed_note, block_header, new_public_notes| { + move |committed_note, block_header, new_public_notes| { Box::pin(on_note_received( store_clone.clone(), - note_updates, committed_note, block_header, new_public_notes, @@ -194,10 +193,9 @@ impl Client { }), Box::new({ let store_clone = self.store.clone(); - move |note_updates, nullifier_update, committed_transactions| { + move |nullifier_update, committed_transactions| { Box::pin(on_nullifier_received( store_clone.clone(), - note_updates, nullifier_update, committed_transactions, )) diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 93938f299..079766a44 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -8,7 +8,6 @@ use miden_objects::{ note::{NoteId, NoteInclusionProof, NoteTag, Nullifier}, Digest, }; -use miden_tx::utils::sync::RwLock; use tracing::info; use super::{block_header::BlockUpdates, get_nullifier_prefix, NoteTagRecord, SyncSummary}; @@ -78,20 +77,19 @@ impl From<&StateSyncUpdate> for SyncSummary { /// TODO: document pub type OnNoteReceived = Box< dyn Fn( - Arc>, CommittedNote, BlockHeader, Arc>, - ) -> Pin>>>, + ) -> Pin>>>, >; /// TODO: document pub type OnNullifierReceived = Box< dyn Fn( - Arc>, NullifierUpdate, Arc>, - ) -> Pin>>>, + ) + -> Pin>>>, >; // STATE SYNC @@ -376,32 +374,37 @@ impl StateSync { .collect(); let mut found_relevant_note = false; - let note_updates = Arc::new(RwLock::new(self.state_sync_update.note_updates.clone())); // TODO: look to remove this clone // Process note inclusions let new_public_notes = Arc::new(self.fetch_public_note_details(&public_note_ids, &block_header).await?); for committed_note in note_inclusions { - let note_is_relevant = (self.on_note_received)( - note_updates.clone(), - committed_note, - block_header, - new_public_notes.clone(), - ) - .await?; + let (new_note_updates, note_is_relevant) = + (self.on_note_received)(committed_note, block_header, new_public_notes.clone()) + .await?; + + self.state_sync_update.note_updates.extend(new_note_updates); found_relevant_note |= note_is_relevant; } // Process nullifiers let committed_transactions = Arc::new(transactions.clone()); for nullifier_update in nullifiers { - let new_transaction_update = (self.on_nullifier_received)( - note_updates.clone(), - nullifier_update, - committed_transactions.clone(), - ) - .await?; + if let Some(input_note_record) = self + .state_sync_update + .note_updates + .get_input_note_by_nullifier(nullifier_update.nullifier) + { + // The note was modified in a previous step so we need to update it again + input_note_record + .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; + } + let (new_note_updates, new_transaction_update) = + (self.on_nullifier_received)(nullifier_update, committed_transactions.clone()) + .await?; + + self.state_sync_update.note_updates.extend(new_note_updates); self.state_sync_update.transaction_updates.extend(new_transaction_update); } @@ -409,8 +412,6 @@ impl StateSync { .transaction_updates .extend(TransactionUpdates::new(transactions, vec![])); - self.state_sync_update.note_updates = note_updates.read().clone(); // TODO: look to remove this clone - Ok(found_relevant_note) } @@ -472,15 +473,15 @@ async fn apply_mmr_changes( /// Default implementation of the [OnNoteReceived] callback. It queries the store for the committed /// note and updates the note records accordingly. If the note is not being tracked, it returns the /// note ID to be queried from the node so it can be queried from the node and tracked. -#[allow(clippy::await_holding_lock)] pub async fn on_note_received( store: Arc, - note_updates: Arc>, committed_note: CommittedNote, block_header: BlockHeader, new_public_notes: Arc>, -) -> Result { - let mut note_updates = note_updates.write(); +) -> Result<(NoteUpdates, bool), ClientError> { + let mut updated_input_notes = vec![]; + let mut updated_output_notes = vec![]; + let inclusion_proof = NoteInclusionProof::new( block_header.block_num(), committed_note.note_index(), @@ -490,35 +491,40 @@ pub async fn on_note_received( let mut is_tracked_note = false; let mut block_is_relevant = false; - note_updates.insert_or_ignore_notes( - &store.get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])).await?, - &store - .get_output_notes(NoteFilter::List(vec![*committed_note.note_id()])) - .await?, - ); - - if let Some(input_note_record) = note_updates.get_input_note_by_id(committed_note.note_id()) { + if let Some(mut input_note_record) = store + .get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])) + .await? + .pop() + { // The note belongs to our locally tracked set of input notes is_tracked_note = true; block_is_relevant = true; //TODO: Check if this is always true input_note_record .inclusion_proof_received(inclusion_proof.clone(), committed_note.metadata())?; input_note_record.block_header_received(block_header)?; + + updated_input_notes.push(input_note_record); // TODO: Only do this if it actually changed } - if let Some(output_note_record) = note_updates.get_output_note_by_id(committed_note.note_id()) { + if let Some(mut output_note_record) = store + .get_output_notes(NoteFilter::List(vec![*committed_note.note_id()])) + .await? + .pop() + { // The note belongs to our locally tracked set of output notes is_tracked_note = true; output_note_record.inclusion_proof_received(inclusion_proof.clone())?; + + updated_output_notes.push(output_note_record); // TODO: Only do this if it actually changed } if !is_tracked_note { - // The note wasn't being tracked but it came in the sync response, it means it matched a - // note tag we are tracking and it needs to be inserted in the store if let Some(public_note) = new_public_notes.iter().find(|note| ¬e.id() == committed_note.note_id()) { - note_updates.insert_or_ignore_notes(&[public_note.clone()], &[]); + // The note wasn't being tracked but it came in the sync response, it means it matched a + // note tag we are tracking and it needs to be inserted in the store + updated_input_notes.push(public_note.clone()); // If the note isn't consumable by the client then the block isn't relevant block_is_relevant = !NoteScreener::new(store) @@ -530,33 +536,27 @@ pub async fn on_note_received( } } - Ok(block_is_relevant) + Ok((NoteUpdates::new(updated_input_notes, updated_output_notes), block_is_relevant)) + //TODO: add insert note functions to note updates } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes /// that match the nullifier and updates the note records accordingly. It also returns the /// transactions that should be discarded as they weren't committed when the nullifier was received. -#[allow(clippy::await_holding_lock)] pub async fn on_nullifier_received( store: Arc, - note_updates: Arc>, nullifier_update: NullifierUpdate, transaction_updates: Arc>, -) -> Result { - let mut note_updates = note_updates.write(); +) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { + let mut updated_input_notes = vec![]; + let mut updated_output_notes = vec![]; + let mut discarded_transactions = vec![]; - note_updates.insert_or_ignore_notes( - &store - .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await?, - &store - .get_output_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await?, - ); - - if let Some(input_note_record) = - note_updates.get_input_note_by_nullifier(nullifier_update.nullifier) + if let Some(mut input_note_record) = store + .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) + .await? + .pop() { if let Some(consumer_transaction) = transaction_updates.iter().find(|t| { input_note_record @@ -578,14 +578,23 @@ pub async fn on_nullifier_received( input_note_record .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } + + updated_input_notes.push(input_note_record); // TODO: Only do this if it actually changed } - if let Some(output_note_record) = - note_updates.get_output_note_by_nullifier(nullifier_update.nullifier) + if let Some(mut output_note_record) = store + .get_output_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) + .await? + .pop() { output_note_record .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)?; + + updated_output_notes.push(output_note_record); // TODO: Only do this if it actually changed } - Ok(TransactionUpdates::new(vec![], discarded_transactions)) + Ok(( + NoteUpdates::new(updated_input_notes, updated_output_notes), + TransactionUpdates::new(vec![], discarded_transactions), + )) } From 53601a6bea45d754979f4a71becee07ab936a47f Mon Sep 17 00:00:00 2001 From: tomyrd Date: Fri, 31 Jan 2025 18:47:25 -0300 Subject: [PATCH 10/14] refactor callbacks --- crates/rust-client/src/note/mod.rs | 13 +++ crates/rust-client/src/sync/state_sync.rs | 117 +++++++++++++--------- crates/rust-client/src/transaction/mod.rs | 5 + 3 files changed, 90 insertions(+), 45 deletions(-) diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 518766729..82a3d3314 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -328,6 +328,19 @@ impl NoteUpdates { self.updated_output_notes.extend(other.updated_output_notes); } + pub fn insert_updates( + &mut self, + input_note: Option, + output_note: Option, + ) { + if let Some(input_note) = input_note { + self.updated_input_notes.insert(input_note.id(), input_note); + } + if let Some(output_note) = output_note { + self.updated_output_notes.insert(output_note.id(), output_note); + } + } + /// Returns a mutable reference to the input note record with the provided nullifier if it /// exists. pub fn get_input_note_by_nullifier( diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 079766a44..54721707c 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -6,6 +6,7 @@ use miden_objects::{ block::{BlockHeader, BlockNumber}, crypto::merkle::{InOrderIndex, MmrDelta, MmrPeaks, PartialMmr}, note::{NoteId, NoteInclusionProof, NoteTag, Nullifier}, + transaction::TransactionId, Digest, }; use tracing::info; @@ -18,7 +19,7 @@ use crate::{ domain::{note::CommittedNote, nullifier::NullifierUpdate, transaction::TransactionUpdate}, NodeRpcClient, }, - store::{InputNoteRecord, NoteFilter, Store, StoreError}, + store::{InputNoteRecord, NoteFilter, OutputNoteRecord, Store, StoreError}, transaction::TransactionUpdates, ClientError, }; @@ -80,7 +81,16 @@ pub type OnNoteReceived = Box< CommittedNote, BlockHeader, Arc>, - ) -> Pin>>>, + ) -> Pin< + Box< + dyn Future< + Output = Result< + (Option, Option, bool), + ClientError, + >, + >, + >, + >, >; /// TODO: document @@ -88,8 +98,16 @@ pub type OnNullifierReceived = Box< dyn Fn( NullifierUpdate, Arc>, - ) - -> Pin>>>, + ) -> Pin< + Box< + dyn Future< + Output = Result< + (Option, Option, Option), + ClientError, + >, + >, + >, + >, >; // STATE SYNC @@ -379,11 +397,13 @@ impl StateSync { let new_public_notes = Arc::new(self.fetch_public_note_details(&public_note_ids, &block_header).await?); for committed_note in note_inclusions { - let (new_note_updates, note_is_relevant) = + let (updated_input_note, updated_output_note, note_is_relevant) = (self.on_note_received)(committed_note, block_header, new_public_notes.clone()) .await?; - self.state_sync_update.note_updates.extend(new_note_updates); + self.state_sync_update + .note_updates + .insert_updates(updated_input_note, updated_output_note); found_relevant_note |= note_is_relevant; } @@ -400,12 +420,17 @@ impl StateSync { .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } - let (new_note_updates, new_transaction_update) = + let (updated_input_note, updated_output_note, discarded_transaction) = (self.on_nullifier_received)(nullifier_update, committed_transactions.clone()) .await?; - self.state_sync_update.note_updates.extend(new_note_updates); - self.state_sync_update.transaction_updates.extend(new_transaction_update); + self.state_sync_update + .note_updates + .insert_updates(updated_input_note, updated_output_note); + + if let Some(transaction_id) = discarded_transaction { + self.state_sync_update.transaction_updates.discarded_transaction(transaction_id); + } } self.state_sync_update @@ -478,9 +503,9 @@ pub async fn on_note_received( committed_note: CommittedNote, block_header: BlockHeader, new_public_notes: Arc>, -) -> Result<(NoteUpdates, bool), ClientError> { - let mut updated_input_notes = vec![]; - let mut updated_output_notes = vec![]; +) -> Result<(Option, Option, bool), ClientError> { + let mut updated_input_note = None; + let mut updated_output_note = None; let inclusion_proof = NoteInclusionProof::new( block_header.block_num(), @@ -498,12 +523,14 @@ pub async fn on_note_received( { // The note belongs to our locally tracked set of input notes is_tracked_note = true; - block_is_relevant = true; //TODO: Check if this is always true - input_note_record + block_is_relevant = true; + let inclusion_proof_received = input_note_record .inclusion_proof_received(inclusion_proof.clone(), committed_note.metadata())?; - input_note_record.block_header_received(block_header)?; + let block_header_received = input_note_record.block_header_received(block_header)?; - updated_input_notes.push(input_note_record); // TODO: Only do this if it actually changed + if inclusion_proof_received || block_header_received { + updated_input_note.replace(input_note_record); + } } if let Some(mut output_note_record) = store @@ -513,9 +540,9 @@ pub async fn on_note_received( { // The note belongs to our locally tracked set of output notes is_tracked_note = true; - output_note_record.inclusion_proof_received(inclusion_proof.clone())?; - - updated_output_notes.push(output_note_record); // TODO: Only do this if it actually changed + if output_note_record.inclusion_proof_received(inclusion_proof.clone())? { + updated_output_note.replace(output_note_record); + } } if !is_tracked_note { @@ -524,7 +551,7 @@ pub async fn on_note_received( { // The note wasn't being tracked but it came in the sync response, it means it matched a // note tag we are tracking and it needs to be inserted in the store - updated_input_notes.push(public_note.clone()); + updated_input_note.replace(public_note.clone()); // If the note isn't consumable by the client then the block isn't relevant block_is_relevant = !NoteScreener::new(store) @@ -536,8 +563,7 @@ pub async fn on_note_received( } } - Ok((NoteUpdates::new(updated_input_notes, updated_output_notes), block_is_relevant)) - //TODO: add insert note functions to note updates + Ok((updated_input_note, updated_output_note, block_is_relevant)) } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes @@ -547,39 +573,42 @@ pub async fn on_nullifier_received( store: Arc, nullifier_update: NullifierUpdate, transaction_updates: Arc>, -) -> Result<(NoteUpdates, TransactionUpdates), ClientError> { - let mut updated_input_notes = vec![]; - let mut updated_output_notes = vec![]; - - let mut discarded_transactions = vec![]; +) -> Result<(Option, Option, Option), ClientError> +{ + let mut updated_input_note = None; + let mut updated_output_note = None; + let mut discarded_transaction = None; if let Some(mut input_note_record) = store .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) .await? .pop() { - if let Some(consumer_transaction) = transaction_updates.iter().find(|t| { - input_note_record - .consumer_transaction_id() - .map_or(false, |id| id == &t.transaction_id) - }) { + let note_changed = if let Some(consumer_transaction) = + transaction_updates.iter().find(|t| { + input_note_record + .consumer_transaction_id() + .map_or(false, |id| id == &t.transaction_id) + }) { // The note was being processed by a local transaction that just got committed input_note_record.transaction_committed( consumer_transaction.transaction_id, consumer_transaction.block_num, - )?; + )? } else { // The note was consumed by an external transaction if let Some(id) = input_note_record.consumer_transaction_id() { // The note was being processed by a local transaction that didn't end up being // committed so it should be discarded - discarded_transactions.push(*id); + discarded_transaction.replace(*id); } input_note_record - .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; - } + .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)? + }; - updated_input_notes.push(input_note_record); // TODO: Only do this if it actually changed + if note_changed { + updated_input_note.replace(input_note_record); + } } if let Some(mut output_note_record) = store @@ -587,14 +616,12 @@ pub async fn on_nullifier_received( .await? .pop() { - output_note_record - .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)?; - - updated_output_notes.push(output_note_record); // TODO: Only do this if it actually changed + if output_note_record + .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)? + { + updated_output_note.replace(output_note_record); + } } - Ok(( - NoteUpdates::new(updated_input_notes, updated_output_notes), - TransactionUpdates::new(vec![], discarded_transactions), - )) + Ok((updated_input_note, updated_output_note, discarded_transaction)) } diff --git a/crates/rust-client/src/transaction/mod.rs b/crates/rust-client/src/transaction/mod.rs index 5bd4b1672..532f3aa23 100644 --- a/crates/rust-client/src/transaction/mod.rs +++ b/crates/rust-client/src/transaction/mod.rs @@ -155,6 +155,11 @@ impl TransactionUpdates { pub fn discarded_transactions(&self) -> &[TransactionId] { &self.discarded_transactions } + + /// Inserts a committed transaction into the transaction updates. + pub fn discarded_transaction(&mut self, transaction_id: TransactionId) { + self.discarded_transactions.push(transaction_id); + } } // TRANSACTION RESULT From 8449debd3ba01a96d8bce5fddfc213ef2a79f3b9 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Fri, 31 Jan 2025 19:02:44 -0300 Subject: [PATCH 11/14] update docs --- crates/rust-client/src/sync/state_sync.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 54721707c..4d8c3cd6b 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -75,7 +75,12 @@ impl From<&StateSyncUpdate> for SyncSummary { // SYNC CALLBACKS // ================================================================================================ -/// TODO: document +/// Callback to be executed when a new note inclusion is received in the sync response. It receives +/// the committed note received from the node, the block header in which the note was included and +/// the list of public notes that were included in the block. +/// +/// It returns two optional notes (one input and one output) that should be updated in the store and +/// a flag indicating if the block is relevant to the client. pub type OnNoteReceived = Box< dyn Fn( CommittedNote, @@ -93,7 +98,12 @@ pub type OnNoteReceived = Box< >, >; -/// TODO: document +/// Callback to be executed when a nullifier is received in the sync response. It receives the +/// nullifier update received from the node and the list of transaction updates that were committed +/// in the block. +/// +/// It returns two optional notes (one input and one output) that should be updated in the store and +/// an optional transaction ID if a transaction should be discarded. pub type OnNullifierReceived = Box< dyn Fn( NullifierUpdate, @@ -496,8 +506,9 @@ async fn apply_mmr_changes( // ================================================================================================ /// Default implementation of the [OnNoteReceived] callback. It queries the store for the committed -/// note and updates the note records accordingly. If the note is not being tracked, it returns the -/// note ID to be queried from the node so it can be queried from the node and tracked. +/// note and updates it accordingly. If the note wasn't being tracked but it came in the sync +/// response, it is also returned so it can be inserted in the store. The method also returns a +/// flag indicating if the block is relevant to the client. pub async fn on_note_received( store: Arc, committed_note: CommittedNote, @@ -567,8 +578,8 @@ pub async fn on_note_received( } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes -/// that match the nullifier and updates the note records accordingly. It also returns the -/// transactions that should be discarded as they weren't committed when the nullifier was received. +/// that match the nullifier and updates the note records accordingly. It also returns an optional +/// transaction ID that should be discarded. pub async fn on_nullifier_received( store: Arc, nullifier_update: NullifierUpdate, From 2daa5133a0fff99b275b6e93ab6753a63f28fc28 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Tue, 4 Feb 2025 14:44:20 -0300 Subject: [PATCH 12/14] change callbacks and move state transitions --- crates/rust-client/src/note/mod.rs | 21 ++ .../rust-client/src/rpc/domain/nullifier.rs | 1 + .../src/store/sqlite_store/sync.rs | 7 +- crates/rust-client/src/sync/mod.rs | 34 +-- crates/rust-client/src/sync/state_sync.rs | 258 +++++++----------- tests/integration/onchain_tests.rs | 20 +- 6 files changed, 157 insertions(+), 184 deletions(-) diff --git a/crates/rust-client/src/note/mod.rs b/crates/rust-client/src/note/mod.rs index 82a3d3314..4f5c0db2f 100644 --- a/crates/rust-client/src/note/mod.rs +++ b/crates/rust-client/src/note/mod.rs @@ -341,6 +341,16 @@ impl NoteUpdates { } } + /// Returns a mutable reference to the input note record with the provided ID if it exists. + pub fn get_input_note_by_id(&mut self, note_id: NoteId) -> Option<&mut InputNoteRecord> { + self.updated_input_notes.get_mut(¬e_id) + } + + /// Returns a mutable reference to the output note record with the provided ID if it exists. + pub fn get_output_note_by_id(&mut self, note_id: NoteId) -> Option<&mut OutputNoteRecord> { + self.updated_output_notes.get_mut(¬e_id) + } + /// Returns a mutable reference to the input note record with the provided nullifier if it /// exists. pub fn get_input_note_by_nullifier( @@ -349,4 +359,15 @@ impl NoteUpdates { ) -> Option<&mut InputNoteRecord> { self.updated_input_notes.values_mut().find(|note| note.nullifier() == nullifier) } + + /// Returns a mutable reference to the output note record with the provided nullifier if it + /// exists. + pub fn get_output_note_by_nullifier( + &mut self, + nullifier: Nullifier, + ) -> Option<&mut OutputNoteRecord> { + self.updated_output_notes + .values_mut() + .find(|note| note.nullifier() == Some(nullifier)) + } } diff --git a/crates/rust-client/src/rpc/domain/nullifier.rs b/crates/rust-client/src/rpc/domain/nullifier.rs index 9834fbdb5..345508063 100644 --- a/crates/rust-client/src/rpc/domain/nullifier.rs +++ b/crates/rust-client/src/rpc/domain/nullifier.rs @@ -6,6 +6,7 @@ use crate::rpc::{errors::RpcConversionError, generated::digest::Digest}; // ================================================================================================ /// Represents a note that was consumed in the node at a certain block. +#[derive(Debug, Clone)] pub struct NullifierUpdate { /// The nullifier of the consumed note. pub nullifier: Nullifier, diff --git a/crates/rust-client/src/store/sqlite_store/sync.rs b/crates/rust-client/src/store/sqlite_store/sync.rs index 655b52127..804bc0412 100644 --- a/crates/rust-client/src/store/sqlite_store/sync.rs +++ b/crates/rust-client/src/store/sqlite_store/sync.rs @@ -101,7 +101,6 @@ impl SqliteStore { note_updates, transaction_updates, account_updates, - tags_to_remove, } = state_sync_update; let mut locked_accounts = vec![]; @@ -141,6 +140,12 @@ impl SqliteStore { apply_note_updates_tx(&tx, ¬e_updates)?; // Remove tags + let tags_to_remove = note_updates.committed_input_notes().map(|note| { + NoteTagRecord::with_note_source( + note.metadata().expect("Committed notes should have metadata").tag(), + note.id(), + ) + }); for tag in tags_to_remove { remove_note_tag_tx(&tx, tag)?; } diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index 3321a3e8c..d472a7db7 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -64,8 +64,9 @@ use miden_objects::{ note::{NoteId, NoteTag, Nullifier}, transaction::TransactionId, }; +use state_sync::on_note_received; -use crate::{Client, ClientError}; +use crate::{store::NoteFilter, Client, ClientError}; mod block_header; @@ -73,10 +74,7 @@ mod tag; pub use tag::{NoteTagRecord, NoteTagSource}; mod state_sync; -pub use state_sync::{ - on_note_received, on_nullifier_received, OnNoteReceived, OnNullifierReceived, StateSync, - StateSyncUpdate, -}; +pub use state_sync::{OnNoteReceived, OnNullifierReceived, StateSync, StateSyncUpdate}; /// Contains stats about the sync operation. pub struct SyncSummary { @@ -182,25 +180,11 @@ impl Client { self.rpc_api.clone(), Box::new({ let store_clone = self.store.clone(); - move |committed_note, block_header, new_public_notes| { - Box::pin(on_note_received( - store_clone.clone(), - committed_note, - block_header, - new_public_notes, - )) - } - }), - Box::new({ - let store_clone = self.store.clone(); - move |nullifier_update, committed_transactions| { - Box::pin(on_nullifier_received( - store_clone.clone(), - nullifier_update, - committed_transactions, - )) + move |committed_note, public_note| { + Box::pin(on_note_received(store_clone.clone(), committed_note, public_note)) } }), + Box::new(move |_committed_note| Box::pin(async { Ok(true) })), ); // Get current state of the client @@ -222,7 +206,8 @@ impl Client { let note_tags: Vec = self.store.get_unique_note_tags().await?.into_iter().collect(); - let unspent_nullifiers = self.store.get_unspent_input_note_nullifiers().await?; + let unspent_input_notes = self.store.get_input_notes(NoteFilter::Unspent).await?; + let unspent_output_notes = self.store.get_output_notes(NoteFilter::Unspent).await?; // Get the sync update from the network let state_sync_update = state_sync @@ -232,7 +217,8 @@ impl Client { self.build_current_partial_mmr(false).await?, accounts, note_tags, - unspent_nullifiers, + unspent_input_notes, + unspent_output_notes, ) .await?; diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 4d8c3cd6b..9fd6982d3 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -11,7 +11,7 @@ use miden_objects::{ }; use tracing::info; -use super::{block_header::BlockUpdates, get_nullifier_prefix, NoteTagRecord, SyncSummary}; +use super::{block_header::BlockUpdates, get_nullifier_prefix, SyncSummary}; use crate::{ account::AccountUpdates, note::{NoteScreener, NoteUpdates}, @@ -40,8 +40,6 @@ pub struct StateSyncUpdate { pub transaction_updates: TransactionUpdates, /// Public account updates and mismatched private accounts after the sync. pub account_updates: AccountUpdates, - /// Tag records that are no longer relevant. - pub tags_to_remove: Vec, } impl From<&StateSyncUpdate> for SyncSummary { @@ -84,18 +82,8 @@ impl From<&StateSyncUpdate> for SyncSummary { pub type OnNoteReceived = Box< dyn Fn( CommittedNote, - BlockHeader, - Arc>, - ) -> Pin< - Box< - dyn Future< - Output = Result< - (Option, Option, bool), - ClientError, - >, - >, - >, - >, + Option, + ) -> Pin>>>, >; /// Callback to be executed when a nullifier is received in the sync response. It receives the @@ -104,21 +92,8 @@ pub type OnNoteReceived = Box< /// /// It returns two optional notes (one input and one output) that should be updated in the store and /// an optional transaction ID if a transaction should be discarded. -pub type OnNullifierReceived = Box< - dyn Fn( - NullifierUpdate, - Arc>, - ) -> Pin< - Box< - dyn Future< - Output = Result< - (Option, Option, Option), - ClientError, - >, - >, - >, - >, ->; +pub type OnNullifierReceived = + Box Pin>>>>; // STATE SYNC // ================================================================================================ @@ -147,7 +122,6 @@ impl StateSync { /// /// * `rpc_api` - The RPC client used to communicate with the node. /// * `on_note_received` - A callback to be executed when a new note inclusion is received. - /// * `on_committed_transaction` - A callback to be executed when a transaction is committed. /// * `on_nullifier_received` - A callback to be executed when a nullifier is received. pub fn new( rpc_api: Arc, @@ -252,8 +226,18 @@ impl StateSync { mut current_partial_mmr: PartialMmr, accounts: Vec, note_tags: Vec, - mut unspent_nullifiers: Vec, + unspent_input_notes: Vec, + unspent_output_notes: Vec, ) -> Result { + let mut unspent_nullifiers: Vec = unspent_input_notes + .iter() + .map(|note| note.nullifier()) + .chain(unspent_output_notes.iter().filter_map(|note| note.nullifier())) + .collect(); + + self.state_sync_update.note_updates = + NoteUpdates::new(unspent_input_notes, unspent_output_notes); + while self .sync_state_step( current_block, @@ -289,21 +273,6 @@ impl StateSync { .expect("At least one block header should be present"); } - // We can remove tags from notes that got committed - let tags_to_remove: Vec = self - .state_sync_update - .note_updates - .committed_input_notes() - .map(|note| { - NoteTagRecord::with_note_source( - note.metadata().expect("Committed note should have metadata").tag(), - note.id(), - ) - }) - .collect(); - - self.state_sync_update.tags_to_remove.extend(tags_to_remove); - Ok(self.state_sync_update) } @@ -407,39 +376,41 @@ impl StateSync { let new_public_notes = Arc::new(self.fetch_public_note_details(&public_note_ids, &block_header).await?); for committed_note in note_inclusions { - let (updated_input_note, updated_output_note, note_is_relevant) = - (self.on_note_received)(committed_note, block_header, new_public_notes.clone()) - .await?; - - self.state_sync_update - .note_updates - .insert_updates(updated_input_note, updated_output_note); - found_relevant_note |= note_is_relevant; + let public_note = new_public_notes + .iter() + .find(|note| ¬e.id() == committed_note.note_id()) + .cloned(); + if (self.on_note_received)(committed_note.clone(), public_note.clone()).await? { + found_relevant_note = true; + + if let Some(public_note) = public_note { + self.state_sync_update.note_updates.insert_updates(Some(public_note), None); + } + + committed_state_transions( + &mut self.state_sync_update.note_updates, + committed_note, + block_header, + ) + .await?; + } } // Process nullifiers - let committed_transactions = Arc::new(transactions.clone()); for nullifier_update in nullifiers { - if let Some(input_note_record) = self - .state_sync_update - .note_updates - .get_input_note_by_nullifier(nullifier_update.nullifier) - { - // The note was modified in a previous step so we need to update it again - input_note_record - .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; - } - - let (updated_input_note, updated_output_note, discarded_transaction) = - (self.on_nullifier_received)(nullifier_update, committed_transactions.clone()) - .await?; - - self.state_sync_update - .note_updates - .insert_updates(updated_input_note, updated_output_note); + if (self.on_nullifier_received)(nullifier_update.clone()).await? { + let discarded_transaction = nullfier_state_transitions( + &mut self.state_sync_update.note_updates, + nullifier_update, + &transactions, + ) + .await?; - if let Some(transaction_id) = discarded_transaction { - self.state_sync_update.transaction_updates.discarded_transaction(transaction_id); + if let Some(transaction_id) = discarded_transaction { + self.state_sync_update + .transaction_updates + .discarded_transaction(transaction_id); + } } } @@ -509,103 +480,56 @@ async fn apply_mmr_changes( /// note and updates it accordingly. If the note wasn't being tracked but it came in the sync /// response, it is also returned so it can be inserted in the store. The method also returns a /// flag indicating if the block is relevant to the client. -pub async fn on_note_received( - store: Arc, +async fn committed_state_transions( + note_updates: &mut NoteUpdates, committed_note: CommittedNote, block_header: BlockHeader, - new_public_notes: Arc>, -) -> Result<(Option, Option, bool), ClientError> { - let mut updated_input_note = None; - let mut updated_output_note = None; - +) -> Result<(), ClientError> { let inclusion_proof = NoteInclusionProof::new( block_header.block_num(), committed_note.note_index(), committed_note.merkle_path().clone(), )?; - let mut is_tracked_note = false; - let mut block_is_relevant = false; - - if let Some(mut input_note_record) = store - .get_input_notes(NoteFilter::List(vec![*committed_note.note_id()])) - .await? - .pop() - { + if let Some(input_note_record) = note_updates.get_input_note_by_id(*committed_note.note_id()) { // The note belongs to our locally tracked set of input notes - is_tracked_note = true; - block_is_relevant = true; - let inclusion_proof_received = input_note_record + input_note_record .inclusion_proof_received(inclusion_proof.clone(), committed_note.metadata())?; - let block_header_received = input_note_record.block_header_received(block_header)?; - - if inclusion_proof_received || block_header_received { - updated_input_note.replace(input_note_record); - } + input_note_record.block_header_received(block_header)?; } - if let Some(mut output_note_record) = store - .get_output_notes(NoteFilter::List(vec![*committed_note.note_id()])) - .await? - .pop() + if let Some(output_note_record) = note_updates.get_output_note_by_id(*committed_note.note_id()) { // The note belongs to our locally tracked set of output notes - is_tracked_note = true; - if output_note_record.inclusion_proof_received(inclusion_proof.clone())? { - updated_output_note.replace(output_note_record); - } - } - - if !is_tracked_note { - if let Some(public_note) = - new_public_notes.iter().find(|note| ¬e.id() == committed_note.note_id()) - { - // The note wasn't being tracked but it came in the sync response, it means it matched a - // note tag we are tracking and it needs to be inserted in the store - updated_input_note.replace(public_note.clone()); - - // If the note isn't consumable by the client then the block isn't relevant - block_is_relevant = !NoteScreener::new(store) - .check_relevance( - &public_note.try_into().expect("Committed notes should have metadata"), - ) - .await? - .is_empty(); - } + output_note_record.inclusion_proof_received(inclusion_proof.clone())?; } - Ok((updated_input_note, updated_output_note, block_is_relevant)) + Ok(()) } /// Default implementation of the [OnNullifierReceived] callback. It queries the store for the notes /// that match the nullifier and updates the note records accordingly. It also returns an optional /// transaction ID that should be discarded. -pub async fn on_nullifier_received( - store: Arc, +async fn nullfier_state_transitions( + note_updates: &mut NoteUpdates, nullifier_update: NullifierUpdate, - transaction_updates: Arc>, -) -> Result<(Option, Option, Option), ClientError> -{ - let mut updated_input_note = None; - let mut updated_output_note = None; + transaction_updates: &[TransactionUpdate], +) -> Result, ClientError> { let mut discarded_transaction = None; - if let Some(mut input_note_record) = store - .get_input_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await? - .pop() + if let Some(input_note_record) = + note_updates.get_input_note_by_nullifier(nullifier_update.nullifier) { - let note_changed = if let Some(consumer_transaction) = - transaction_updates.iter().find(|t| { - input_note_record - .consumer_transaction_id() - .map_or(false, |id| id == &t.transaction_id) - }) { + if let Some(consumer_transaction) = transaction_updates.iter().find(|t| { + input_note_record + .consumer_transaction_id() + .map_or(false, |id| id == &t.transaction_id) + }) { // The note was being processed by a local transaction that just got committed input_note_record.transaction_committed( consumer_transaction.transaction_id, consumer_transaction.block_num, - )? + )?; } else { // The note was consumed by an external transaction if let Some(id) = input_note_record.consumer_transaction_id() { @@ -614,25 +538,43 @@ pub async fn on_nullifier_received( discarded_transaction.replace(*id); } input_note_record - .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)? - }; - - if note_changed { - updated_input_note.replace(input_note_record); + .consumed_externally(nullifier_update.nullifier, nullifier_update.block_num)?; } } - if let Some(mut output_note_record) = store - .get_output_notes(NoteFilter::Nullifiers(vec![nullifier_update.nullifier])) - .await? - .pop() + if let Some(output_note_record) = + note_updates.get_output_note_by_nullifier(nullifier_update.nullifier) { - if output_note_record - .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)? - { - updated_output_note.replace(output_note_record); - } + output_note_record + .nullifier_received(nullifier_update.nullifier, nullifier_update.block_num)?; } - Ok((updated_input_note, updated_output_note, discarded_transaction)) + Ok(discarded_transaction) +} + +pub async fn on_note_received( + store: Arc, + committed_note: CommittedNote, + public_note: Option, +) -> Result { + let note_id = *committed_note.note_id(); + let note_screener = NoteScreener::new(store.clone()); + + if !store.get_input_notes(NoteFilter::Unique(note_id)).await?.is_empty() + || !store.get_output_notes(NoteFilter::Unique(note_id)).await?.is_empty() + { + // The note is being tracked by the client so it is relevant + Ok(true) + } else if let Some(public_note) = public_note { + // The note is not being tracked by the client and is public so we can screen it + let new_note_relevance = note_screener + .check_relevance(&public_note.try_into().expect("Public notes should contain metadata")) + .await?; + + Ok(!new_note_relevance.is_empty()) + } else { + // The note is not being tracked by the client and is private so we can't determine if it + // is relevant + Ok(false) + } } diff --git a/tests/integration/onchain_tests.rs b/tests/integration/onchain_tests.rs index 3b29f8b28..3e2dc5cef 100644 --- a/tests/integration/onchain_tests.rs +++ b/tests/integration/onchain_tests.rs @@ -83,8 +83,26 @@ async fn test_onchain_notes_flow() { .build(); execute_tx_and_sync(&mut client_2, basic_wallet_1.id(), tx_request).await; + // Create a note for client 3 that is already consumed before syncing + let tx_request = TransactionRequestBuilder::pay_to_id( + PaymentTransactionData::new( + vec![p2id_asset.into()], + basic_wallet_1.id(), + basic_wallet_2.id(), + ), + Some(1.into()), + NoteType::Public, + client_2.rng(), + ) + .unwrap() + .build(); + let note = tx_request.expected_output_notes().next().unwrap().clone(); + execute_tx_and_sync(&mut client_2, basic_wallet_1.id(), tx_request).await; + + let tx_request = TransactionRequestBuilder::consume_notes(vec![note.id()]).build(); + execute_tx_and_sync(&mut client_2, basic_wallet_1.id(), tx_request).await; + // sync client 3 (basic account 2) - client_3.add_note_tag(note.metadata().tag()).await.unwrap(); client_3.sync_state().await.unwrap(); // client 3 should have two notes, the one directed to them and the one consumed by client 2 From c169f7cde7b9c6a3b101d1cedc4ea6ad0bd09431 Mon Sep 17 00:00:00 2001 From: tomyrd Date: Tue, 4 Feb 2025 16:52:31 -0300 Subject: [PATCH 13/14] fix: web store --- crates/rust-client/src/store/web_store/sync/mod.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/crates/rust-client/src/store/web_store/sync/mod.rs b/crates/rust-client/src/store/web_store/sync/mod.rs index 23a5eaf9c..f847d8754 100644 --- a/crates/rust-client/src/store/web_store/sync/mod.rs +++ b/crates/rust-client/src/store/web_store/sync/mod.rs @@ -111,7 +111,6 @@ impl WebStore { note_updates, transaction_updates, //TODO: Add support for discarded transactions in web store account_updates, - tags_to_remove, } = state_sync_update; // Serialize data for updating state sync and block header @@ -144,16 +143,8 @@ impl WebStore { apply_note_updates_tx(¬e_updates).await?; // Tags to remove - let note_tags_to_remove_as_str: Vec = tags_to_remove - .iter() - .filter_map(|tag_record| { - if let NoteTagSource::Note(note_id) = tag_record.source { - Some(note_id.to_hex()) - } else { - None - } - }) - .collect(); + let note_tags_to_remove_as_str: Vec = + note_updates.committed_input_notes().map(|note| note.id().to_hex()).collect(); // Serialize data for updating committed transactions let transactions_to_commit_block_nums_as_str = transaction_updates From 7d0f943c78e24469b72cda9f51d18d52b486b28c Mon Sep 17 00:00:00 2001 From: tomyrd Date: Tue, 4 Feb 2025 17:05:21 -0300 Subject: [PATCH 14/14] refator: use partial mmr with current block in sync --- crates/rust-client/src/note/import.rs | 4 +- crates/rust-client/src/sync/block_header.rs | 23 ++++----- crates/rust-client/src/sync/mod.rs | 11 +---- crates/rust-client/src/sync/state_sync.rs | 55 +++++++-------------- crates/rust-client/src/tests.rs | 2 +- crates/rust-client/src/transaction/mod.rs | 2 +- 6 files changed, 32 insertions(+), 65 deletions(-) diff --git a/crates/rust-client/src/note/import.rs b/crates/rust-client/src/note/import.rs index 3e716d518..a18cee577 100644 --- a/crates/rust-client/src/note/import.rs +++ b/crates/rust-client/src/note/import.rs @@ -175,7 +175,7 @@ impl Client { note_record.inclusion_proof_received(inclusion_proof, metadata)?; if block_height < current_block_num { - let mut current_partial_mmr = self.build_current_partial_mmr(true).await?; + let mut current_partial_mmr = self.build_current_partial_mmr().await?; let block_header = self .get_and_store_authenticated_block(block_height, &mut current_partial_mmr) @@ -219,7 +219,7 @@ impl Client { match committed_note_data { Some((metadata, inclusion_proof)) => { - let mut current_partial_mmr = self.build_current_partial_mmr(true).await?; + let mut current_partial_mmr = self.build_current_partial_mmr().await?; let block_header = self .get_and_store_authenticated_block( inclusion_proof.location().block_num(), diff --git a/crates/rust-client/src/sync/block_header.rs b/crates/rust-client/src/sync/block_header.rs index 76fe9940b..e0a2d44b4 100644 --- a/crates/rust-client/src/sync/block_header.rs +++ b/crates/rust-client/src/sync/block_header.rs @@ -37,7 +37,7 @@ impl Client { /// Updates committed notes with no MMR data. These could be notes that were /// imported with an inclusion proof, but its block header isn't tracked. pub(crate) async fn update_mmr_data(&self) -> Result<(), ClientError> { - let mut current_partial_mmr = self.build_current_partial_mmr(true).await?; + let mut current_partial_mmr = self.build_current_partial_mmr().await?; let mut changed_notes = vec![]; for mut note in self.store.get_input_notes(NoteFilter::Unverified).await? { @@ -96,10 +96,7 @@ impl Client { /// /// As part of the syncing process, we add the current block number so we don't need to /// track it here. - pub(crate) async fn build_current_partial_mmr( - &self, - include_current_block: bool, - ) -> Result { + pub(crate) async fn build_current_partial_mmr(&self) -> Result { let current_block_num = self.store.get_sync_height().await?; let tracked_nodes = self.store.get_chain_mmr_nodes(ChainMmrNodeFilter::All).await?; @@ -121,15 +118,13 @@ impl Client { let mut current_partial_mmr = PartialMmr::from_parts(current_peaks, tracked_nodes, track_latest); - if include_current_block { - let (current_block, has_client_notes) = self - .store - .get_block_header_by_num(current_block_num) - .await? - .expect("Current block should be in the store"); + let (current_block, has_client_notes) = self + .store + .get_block_header_by_num(current_block_num) + .await? + .expect("Current block should be in the store"); - current_partial_mmr.add(current_block.hash(), has_client_notes); - } + current_partial_mmr.add(current_block.hash(), has_client_notes); Ok(current_partial_mmr) } @@ -197,7 +192,7 @@ impl Client { return self.ensure_genesis_in_place().await; } - let mut current_partial_mmr = self.build_current_partial_mmr(true).await?; + let mut current_partial_mmr = self.build_current_partial_mmr().await?; let anchor_block = self .get_and_store_authenticated_block(epoch_block_number, &mut current_partial_mmr) .await?; diff --git a/crates/rust-client/src/sync/mod.rs b/crates/rust-client/src/sync/mod.rs index d472a7db7..644202c25 100644 --- a/crates/rust-client/src/sync/mod.rs +++ b/crates/rust-client/src/sync/mod.rs @@ -188,13 +188,6 @@ impl Client { ); // Get current state of the client - let current_block_num = self.store.get_sync_height().await?; - let (current_block, has_relevant_notes) = self - .store - .get_block_header_by_num(current_block_num) - .await? - .expect("Current block should be in the store"); - let accounts = self .store .get_account_headers() @@ -212,9 +205,7 @@ impl Client { // Get the sync update from the network let state_sync_update = state_sync .sync_state( - current_block, - has_relevant_notes, - self.build_current_partial_mmr(false).await?, + self.build_current_partial_mmr().await?, accounts, note_tags, unspent_input_notes, diff --git a/crates/rust-client/src/sync/state_sync.rs b/crates/rust-client/src/sync/state_sync.rs index 9fd6982d3..20beb2510 100644 --- a/crates/rust-client/src/sync/state_sync.rs +++ b/crates/rust-client/src/sync/state_sync.rs @@ -147,14 +147,12 @@ impl StateSync { /// step. async fn sync_state_step( &mut self, - current_block: BlockHeader, - current_block_has_relevant_notes: bool, current_partial_mmr: &mut PartialMmr, accounts: &[AccountHeader], note_tags: &[NoteTag], unspent_nullifiers: &[Nullifier], ) -> Result { - let current_block_num = current_block.block_num(); + let current_block_num = (current_partial_mmr.num_leaves() as u32 - 1).into(); let account_ids: Vec = accounts.iter().map(|acc| acc.id()).collect(); // To receive information about added nullifiers, we reduce them to the higher 16 bits @@ -188,8 +186,8 @@ impl StateSync { .await?; let (new_mmr_peaks, new_authentication_nodes) = apply_mmr_changes( - current_block, - current_block_has_relevant_notes, + response.block_header, + found_relevant_note, current_partial_mmr, response.mmr_delta, ) @@ -221,8 +219,6 @@ impl StateSync { /// * `unspent_nullifiers` - The nullifiers of tracked notes that haven't been consumed. pub async fn sync_state( mut self, - mut current_block: BlockHeader, - mut current_block_has_relevant_notes: bool, mut current_partial_mmr: PartialMmr, accounts: Vec, note_tags: Vec, @@ -239,14 +235,7 @@ impl StateSync { NoteUpdates::new(unspent_input_notes, unspent_output_notes); while self - .sync_state_step( - current_block, - current_block_has_relevant_notes, - &mut current_partial_mmr, - &accounts, - ¬e_tags, - &unspent_nullifiers, - ) + .sync_state_step(&mut current_partial_mmr, &accounts, ¬e_tags, &unspent_nullifiers) .await? { // New nullfiers should be added for new untracked notes that were added in previous @@ -262,15 +251,6 @@ impl StateSync { .map(|note| note.nullifier()) .collect::>(), ); - - // Update the current block for the next step - (current_block, current_block_has_relevant_notes, ..) = self - .state_sync_update - .block_updates - .block_headers - .last() - .cloned() - .expect("At least one block header should be present"); } Ok(self.state_sync_update) @@ -451,26 +431,27 @@ impl StateSync { /// Applies changes to the current MMR structure, returns the updated [MmrPeaks] and the /// authentication nodes for leaves we track. async fn apply_mmr_changes( - current_block: BlockHeader, - current_block_has_relevant_notes: bool, + new_block: BlockHeader, + new_block_has_relevant_notes: bool, current_partial_mmr: &mut PartialMmr, mmr_delta: MmrDelta, ) -> Result<(MmrPeaks, Vec<(InOrderIndex, Digest)>), ClientError> { // First, apply curent_block to the MMR. This is needed as the MMR delta received from the // node doesn't contain the request block itself. - let new_authentication_nodes = current_partial_mmr - .add(current_block.hash(), current_block_has_relevant_notes) - .into_iter(); + // let new_authentication_nodes = current_partial_mmr + // .add(current_block.hash(), current_block_has_relevant_notes) + // .into_iter(); // Apply the MMR delta to bring MMR to forest equal to chain tip - let new_authentication_nodes: Vec<(InOrderIndex, Digest)> = current_partial_mmr - .apply(mmr_delta) - .map_err(StoreError::MmrError)? - .into_iter() - .chain(new_authentication_nodes) - .collect(); - - Ok((current_partial_mmr.peaks(), new_authentication_nodes)) + let mut new_authentication_nodes: Vec<(InOrderIndex, Digest)> = + current_partial_mmr.apply(mmr_delta).map_err(StoreError::MmrError)?; + + let new_peaks = current_partial_mmr.peaks(); + + new_authentication_nodes + .append(&mut current_partial_mmr.add(new_block.hash(), new_block_has_relevant_notes)); + + Ok((new_peaks, new_authentication_nodes)) } // DEFAULT CALLBACK IMPLEMENTATIONS diff --git a/crates/rust-client/src/tests.rs b/crates/rust-client/src/tests.rs index babad8f49..2239bfc9d 100644 --- a/crates/rust-client/src/tests.rs +++ b/crates/rust-client/src/tests.rs @@ -374,7 +374,7 @@ async fn test_sync_state_mmr() { ); // Try reconstructing the chain_mmr from what's in the database - let partial_mmr = client.build_current_partial_mmr(true).await.unwrap(); + let partial_mmr = client.build_current_partial_mmr().await.unwrap(); assert_eq!(partial_mmr.forest(), 6); assert!(partial_mmr.open(0).unwrap().is_some()); // Account anchor block assert!(partial_mmr.open(1).unwrap().is_some()); diff --git a/crates/rust-client/src/transaction/mod.rs b/crates/rust-client/src/transaction/mod.rs index 532f3aa23..e9ab67438 100644 --- a/crates/rust-client/src/transaction/mod.rs +++ b/crates/rust-client/src/transaction/mod.rs @@ -913,7 +913,7 @@ impl Client { let summary = self.sync_state().await?; if summary.block_num != block_num { - let mut current_partial_mmr = self.build_current_partial_mmr(true).await?; + let mut current_partial_mmr = self.build_current_partial_mmr().await?; self.get_and_store_authenticated_block(block_num, &mut current_partial_mmr) .await?; }