diff --git a/bin/integration-tests/src/tests/client.rs b/bin/integration-tests/src/tests/client.rs index adc29818b..95790341c 100644 --- a/bin/integration-tests/src/tests/client.rs +++ b/bin/integration-tests/src/tests/client.rs @@ -218,10 +218,14 @@ pub async fn test_import_expected_notes(client_config: ClientConfig) -> Result<( client_2.sync_state().await.unwrap(); // Importing a public note before it's committed onchain should fail - assert!(matches!( - client_2.import_note(NoteFile::NoteId(note.id())).await.unwrap_err(), - ClientError::NoteNotFoundOnChain(_) - )); + assert_eq!( + client_2 + .import_notes(&[NoteFile::NoteId(note.id())]) + .await + .unwrap_err() + .to_string(), + "note import error: No notes fetched from node".to_string() + ); execute_tx_and_sync(&mut client_1, faucet_account.id(), tx_request).await?; // Use client 1 to wait until a couple of blocks have passed @@ -230,7 +234,7 @@ pub async fn test_import_expected_notes(client_config: ClientConfig) -> Result<( let new_sync_data = client_2.sync_state().await.unwrap(); client_2.add_note_tag(note.metadata().unwrap().tag()).await.unwrap(); - client_2.import_note(NoteFile::NoteId(note.clone().id())).await.unwrap(); + client_2.import_notes(&[NoteFile::NoteId(note.clone().id())]).await.unwrap(); client_2.sync_state().await.unwrap(); let input_note = client_2.get_input_note(note.id()).await.unwrap().unwrap(); // If imported after execution and syncing then the inclusion proof should be Some @@ -260,11 +264,11 @@ pub async fn test_import_expected_notes(client_config: ClientConfig) -> Result<( // Import the node before it's committed onchain works if we have full `NoteDetails` client_2.add_note_tag(note.metadata().unwrap().tag()).await.unwrap(); client_2 - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.clone().into(), after_block_num: client_1.get_sync_height().await.unwrap(), tag: Some(note.metadata().unwrap().tag()), - }) + }]) .await .unwrap(); let input_note = client_2.get_input_note(note.id()).await.unwrap().unwrap(); @@ -327,12 +331,12 @@ pub async fn test_import_expected_note_uncommitted(client_config: ClientConfig) // If the verification is requested before execution then the import should fail let imported_note_id = client_2 - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.into(), after_block_num: 0.into(), tag: None, - }) - .await?; + }]) + .await?[0]; let imported_note = client_2.get_input_note(imported_note_id).await.unwrap().unwrap(); @@ -377,12 +381,12 @@ pub async fn test_import_expected_notes_from_the_past_as_committed( // importing the note before client_2 is synced will result in a note with `Expected` state let note_id = client_2 - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.clone().into(), after_block_num: block_height_before, tag: Some(note.metadata().unwrap().tag()), - }) - .await?; + }]) + .await?[0]; let imported_note = client_2.get_input_note(note_id).await.unwrap().unwrap(); @@ -390,14 +394,17 @@ pub async fn test_import_expected_notes_from_the_past_as_committed( client_2.sync_state().await.unwrap(); - // import the note after syncing the client - let note_id = client_2 - .import_note(NoteFile::NoteDetails { - details: note.clone().into(), - after_block_num: block_height_before, - tag: Some(note.metadata().unwrap().tag()), - }) - .await?; + // Note already imported + assert!( + client_2 + .import_notes(&[NoteFile::NoteDetails { + details: note.clone().into(), + after_block_num: block_height_before, + tag: Some(note.metadata().unwrap().tag()), + }]) + .await? + .is_empty() + ); let imported_note = client_2.get_input_note(note_id).await.unwrap().unwrap(); @@ -835,10 +842,10 @@ pub async fn test_import_consumed_note_with_proof(client_config: ClientConfig) - // Import the consumed note client_2 - .import_note(NoteFile::NoteWithProof( + .import_notes(&[NoteFile::NoteWithProof( note.clone().try_into().unwrap(), note.inclusion_proof().unwrap().clone(), - )) + )]) .await?; let consumed_note = client_2.get_input_note(note.id()).await.unwrap().unwrap(); @@ -900,7 +907,7 @@ pub async fn test_import_consumed_note_with_id(client_config: ClientConfig) -> R client_2.sync_state().await.unwrap(); // Import the consumed note - client_2.import_note(NoteFile::NoteId(note.id())).await.unwrap(); + client_2.import_notes(&[NoteFile::NoteId(note.id())]).await.unwrap(); let consumed_note = client_2.get_input_note(note.id()).await.unwrap().unwrap(); assert!(matches!(consumed_note.state(), InputNoteState::ConsumedExternal { .. })); @@ -956,10 +963,10 @@ pub async fn test_import_note_with_proof(client_config: ClientConfig) -> Result< // Import the consumed note client_2 - .import_note(NoteFile::NoteWithProof( + .import_notes(&[NoteFile::NoteWithProof( note.clone().try_into().unwrap(), note.inclusion_proof().unwrap().clone(), - )) + )]) .await?; let imported_note = client_2.get_input_note(note.id()).await.unwrap().unwrap(); diff --git a/bin/integration-tests/src/tests/onchain.rs b/bin/integration-tests/src/tests/onchain.rs index 63b2422ab..ff9fee1d8 100644 --- a/bin/integration-tests/src/tests/onchain.rs +++ b/bin/integration-tests/src/tests/onchain.rs @@ -297,7 +297,7 @@ pub async fn test_onchain_accounts(client_config: ClientConfig) -> Result<()> { let notes = client_2.get_input_notes(NoteFilter::Committed).await?; //Import the note on the first client so that we can later check its consumer account - client_1.import_note(NoteFile::NoteId(notes[0].id())).await?; + client_1.import_notes(&[NoteFile::NoteId(notes[0].id())]).await?; // Consume the note println!("Consuming note on second client..."); diff --git a/bin/integration-tests/src/tests/pass_through.rs b/bin/integration-tests/src/tests/pass_through.rs index 12fa97e1a..248d5f165 100644 --- a/bin/integration-tests/src/tests/pass_through.rs +++ b/bin/integration-tests/src/tests/pass_through.rs @@ -109,8 +109,12 @@ pub async fn test_pass_through(client_config: ClientConfig) -> Result<()> { println!("consuming pass-through note"); - client.import_note(NoteFile::NoteId(pass_through_note_1.id())).await?; - client.import_note(NoteFile::NoteId(pass_through_note_2.id())).await?; + client + .import_notes(&[ + NoteFile::NoteId(pass_through_note_1.id()), + NoteFile::NoteId(pass_through_note_2.id()), + ]) + .await?; client.sync_state().await?; let input_note_record = client.get_input_note(pass_through_note_1.id()).await?.unwrap(); assert!(matches!(input_note_record.state(), InputNoteState::Committed { .. })); diff --git a/bin/integration-tests/src/tests/swap_transaction.rs b/bin/integration-tests/src/tests/swap_transaction.rs index 0ebd0db8a..6ed7447f4 100644 --- a/bin/integration-tests/src/tests/swap_transaction.rs +++ b/bin/integration-tests/src/tests/swap_transaction.rs @@ -309,11 +309,11 @@ pub async fn test_swap_private(client_config: ClientConfig) -> Result<()> { )?; client2.add_note_tag(tag).await?; client2 - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: output_note.try_into()?, after_block_num: client1.get_sync_height().await?, tag: Some(tag), - }) + }]) .await?; // Sync so we get the inclusion proof info diff --git a/bin/miden-cli/src/commands/import.rs b/bin/miden-cli/src/commands/import.rs index 310c4fa5a..6048ab58a 100644 --- a/bin/miden-cli/src/commands/import.rs +++ b/bin/miden-cli/src/commands/import.rs @@ -35,7 +35,7 @@ impl ImportCmd { let note_file = read_note_file(filename.clone()); if let Ok(note_file) = note_file { - let note_id = client.import_note(note_file).await?; + let note_id = client.import_notes(&[note_file]).await?[0]; println!("Successfully imported note {}", note_id.to_hex()); } else { info!( diff --git a/crates/rust-client/src/note/import.rs b/crates/rust-client/src/note/import.rs index c51de25a2..ec286a8bf 100644 --- a/crates/rust-client/src/note/import.rs +++ b/crates/rust-client/src/note/import.rs @@ -8,7 +8,9 @@ //! //! For more specific information on how the process is performed, refer to the docs for //! [`Client::import_note()`]. +use alloc::collections::{BTreeMap, BTreeSet}; use alloc::string::ToString; +use alloc::vec::Vec; use miden_protocol::block::BlockNumber; use miden_protocol::note::{ @@ -25,7 +27,7 @@ use miden_tx::auth::TransactionAuthenticator; use crate::rpc::RpcError; use crate::rpc::domain::note::FetchedNote; use crate::store::input_note_states::ExpectedNoteState; -use crate::store::{InputNoteRecord, InputNoteState}; +use crate::store::{InputNoteRecord, InputNoteState, NoteFilter}; use crate::sync::NoteTagRecord; use crate::{Client, ClientError}; @@ -37,15 +39,16 @@ where // INPUT NOTE CREATION // -------------------------------------------------------------------------------------------- - /// Imports a new input note into the client's store. The information stored depends on the - /// type of note file provided. If the note existed previously, it will be updated with the - /// new information. The tag specified by the `NoteFile` will start being tracked. + /// Imports a batch of new input notes into the client's store. The information stored depends + /// on the type of note files provided. If the notes existed previously, it will be updated + /// with the new information. The tags specified by the `NoteFile`s will start being + /// tracked. Returns the IDs of notes that were successfully imported or updated. /// - /// - If the note file is a [`NoteFile::NoteId`], the note is fetched from the node and stored + /// - If the note files are [`NoteFile::NoteId`], the notes are fetched from the node and stored /// in the client's store. If the note is private or doesn't exist, an error is returned. - /// - If the note file is a [`NoteFile::NoteDetails`], a new note is created with the provided - /// details and tag. - /// - If the note file is a [`NoteFile::NoteWithProof`], the note is stored with the provided + /// - If the note files are [`NoteFile::NoteDetails`], new notes are created with the provided + /// details and tags. + /// - If the note files are [`NoteFile::NoteWithProof`], the notes are stored with the provided /// inclusion proof and metadata. The block header data is only fetched from the node if the /// note is committed in the past relative to the client. /// @@ -54,34 +57,76 @@ where /// - If an attempt is made to overwrite a note that is currently processing. /// - If the client has reached the note tags limit /// ([`NOTE_TAG_LIMIT`](crate::rpc::NOTE_TAG_LIMIT)). - pub async fn import_note(&mut self, note_file: NoteFile) -> Result { - let id = match ¬e_file { - NoteFile::NoteId(id) => *id, - NoteFile::NoteDetails { details, .. } => details.id(), - NoteFile::NoteWithProof(note, _) => note.id(), - }; - - let previous_note = self.get_input_note(id).await?; - - // If the note is already in the store and is in the state processing we return an error. - if let Some(true) = previous_note.as_ref().map(InputNoteRecord::is_processing) { - return Err(ClientError::NoteImportError(format!( - "Can't overwrite note with id {id} as it's currently being processed", - ))); + /// + /// Note: This operation is atomic. If any note file is invalid or any existing note is in the + /// processing state, the entire operation fails and no notes are imported. + pub async fn import_notes( + &mut self, + note_files: &[NoteFile], + ) -> Result, ClientError> { + let mut note_ids_map = BTreeMap::new(); + for note_file in note_files { + let id = match ¬e_file { + NoteFile::NoteId(id) => *id, + NoteFile::NoteDetails { details, .. } => details.id(), + NoteFile::NoteWithProof(note, _) => note.id(), + }; + note_ids_map.insert(id, note_file); + } + + let note_ids: Vec = note_ids_map.keys().copied().collect(); + let previous_notes: Vec = + self.get_input_notes(NoteFilter::List(note_ids)).await?; + let previous_notes_map: BTreeMap = + previous_notes.into_iter().map(|note| (note.id(), note)).collect(); + + let mut requests_by_id = BTreeMap::new(); + let mut requests_by_details = vec![]; + let mut requests_by_proof = vec![]; + + for (note_id, note_file) in note_ids_map { + let previous_note = previous_notes_map.get(¬e_id).cloned(); + + // If the note is already in the store and is in the state processing we return an + // error. + if let Some(true) = previous_note.as_ref().map(InputNoteRecord::is_processing) { + return Err(ClientError::NoteImportError(format!( + "Can't overwrite note with id {note_id} as it's currently being processed", + ))); + } + + match note_file.clone() { + NoteFile::NoteId(id) => { + requests_by_id.insert(id, previous_note); + }, + NoteFile::NoteDetails { details, after_block_num, tag } => { + requests_by_details.push((previous_note, details, after_block_num, tag)); + }, + NoteFile::NoteWithProof(note, inclusion_proof) => { + requests_by_proof.push((previous_note, note, inclusion_proof)); + }, + } + } + + let mut imported_notes = vec![]; + if !requests_by_id.is_empty() { + let notes_by_id = self.import_note_records_by_id(requests_by_id).await?; + imported_notes.extend(notes_by_id.values().cloned()); + } + + if !requests_by_details.is_empty() { + let notes_by_details = self.import_note_records_by_details(requests_by_details).await?; + imported_notes.extend(notes_by_details); + } + + if !requests_by_proof.is_empty() { + let notes_by_proof = self.import_note_records_by_proof(requests_by_proof).await?; + imported_notes.extend(notes_by_proof); } - let note = match note_file { - NoteFile::NoteId(id) => self.import_note_record_by_id(previous_note, id).await?, - NoteFile::NoteDetails { details, after_block_num, tag } => { - self.import_note_record_by_details(previous_note, details, after_block_num, tag) - .await? - }, - NoteFile::NoteWithProof(note, inclusion_proof) => { - self.import_note_record_by_proof(previous_note, note, inclusion_proof).await? - }, - }; - - if let Some(note) = note { + let mut imported_note_ids = vec![]; + for note in imported_notes.into_iter().flatten() { + imported_note_ids.push(note.id()); if let InputNoteState::Expected(ExpectedNoteState { tag: Some(tag), .. }) = note.state() { self.insert_note_tag(NoteTagRecord::with_note_source(*tag, note.id())).await?; @@ -89,222 +134,290 @@ where self.store.upsert_input_notes(&[note]).await?; } - Ok(id) + Ok(imported_note_ids) } // HELPERS // ================================================================================================ - /// Builds a note record from the note ID. If a note with the same ID was already stored it is - /// passed via `previous_note` so it can be updated. The note information is fetched from the - /// node and stored in the client's store. + /// Builds a note record map from the note IDs. If a note with the same ID was already stored it + /// is passed via `previous_note` so it can be updated. The note information is fetched from + /// the node and stored in the client's store. /// /// # Errors: - /// - If the note doesn't exist on the node. - /// - If the note exists but is private. - async fn import_note_record_by_id( + /// - If a note doesn't exist on the node. + /// - If a note exists but is private. + async fn import_note_records_by_id( &self, - previous_note: Option, - id: NoteId, - ) -> Result, ClientError> { - let fetched_note = self.rpc_api.get_note_by_id(id).await.map_err(|err| match err { - RpcError::NoteNotFound(note_id) => ClientError::NoteNotFoundOnChain(note_id), - err => ClientError::RpcError(err), - })?; - - let inclusion_proof = fetched_note.inclusion_proof().clone(); - - if let Some(mut previous_note) = previous_note { - if previous_note.inclusion_proof_received(inclusion_proof, *fetched_note.metadata())? { - self.store.remove_note_tag((&previous_note).try_into()?).await?; + notes: BTreeMap>, + ) -> Result>, ClientError> { + let note_ids = notes.keys().copied().collect::>(); + + let fetched_notes = + self.rpc_api.get_notes_by_id(¬e_ids).await.map_err(|err| match err { + RpcError::NoteNotFound(note_id) => ClientError::NoteNotFoundOnChain(note_id), + err => ClientError::RpcError(err), + })?; + + if fetched_notes.is_empty() { + return Err(ClientError::NoteImportError("No notes fetched from node".to_string())); + } - Ok(Some(previous_note)) + let mut note_records = BTreeMap::new(); + let mut notes_to_request = vec![]; + for fetched_note in fetched_notes { + let note_id = fetched_note.id(); + let inclusion_proof = fetched_note.inclusion_proof().clone(); + + let previous_note = + notes.get(¬e_id).cloned().ok_or(ClientError::NoteImportError(format!( + "Failed to retrieve note with id {note_id} from node" + )))?; + if let Some(mut previous_note) = previous_note { + if previous_note + .inclusion_proof_received(inclusion_proof, *fetched_note.metadata())? + { + self.store.remove_note_tag((&previous_note).try_into()?).await?; + + note_records.insert(note_id, Some(previous_note)); + } else { + note_records.insert(note_id, None); + } } else { - Ok(None) + let fetched_note = match fetched_note { + FetchedNote::Public(note, _) => note, + FetchedNote::Private(..) => { + return Err(ClientError::NoteImportError( + "Incomplete imported note is private".to_string(), + )); + }, + }; + + let note_request = (previous_note, fetched_note, inclusion_proof); + notes_to_request.push(note_request); } - } else { - let fetched_note = match fetched_note { - FetchedNote::Public(note, _) => note, - FetchedNote::Private(..) => { - return Err(ClientError::NoteImportError( - "Incomplete imported note is private".to_string(), - )); - }, - }; + } - self.import_note_record_by_proof(previous_note, fetched_note, inclusion_proof) - .await + if !notes_to_request.is_empty() { + let note_records_by_proof = self.import_note_records_by_proof(notes_to_request).await?; + for note_record in note_records_by_proof.iter().flatten().cloned() { + note_records.insert(note_record.id(), Some(note_record)); + } } + Ok(note_records) } - /// Builds a note record from the note and inclusion proof. If a note with the same ID was + /// Builds a note record list from notes and inclusion proofs. If a note with the same ID was /// already stored it is passed via `previous_note` so it can be updated. The note's /// nullifier is used to determine if the note has been consumed in the node and gives it /// the correct state. /// /// If the note isn't consumed and it was committed in the past relative to the client, then /// the MMR for the relevant block is fetched from the node and stored. - pub(crate) async fn import_note_record_by_proof( + pub(crate) async fn import_note_records_by_proof( &self, - previous_note: Option, - note: Note, - inclusion_proof: NoteInclusionProof, - ) -> Result, ClientError> { - let metadata = *note.metadata(); - let mut note_record = previous_note.unwrap_or(InputNoteRecord::new( - note.into(), - self.store.get_current_timestamp(), - ExpectedNoteState { - metadata: Some(metadata), - after_block_num: inclusion_proof.location().block_num(), - tag: Some(metadata.tag()), + requested_notes: Vec<(Option, Note, NoteInclusionProof)>, + ) -> Result>, ClientError> { + // TODO: iterating twice over requested notes + let mut note_records = vec![]; + + let mut nullifier_requests = BTreeSet::new(); + let mut lowest_block_height: BlockNumber = u32::MAX.into(); + for (previous_note, note, inclusion_proof) in &requested_notes { + if let Some(previous_note) = previous_note { + nullifier_requests.insert(previous_note.nullifier()); + if inclusion_proof.location().block_num() < lowest_block_height { + lowest_block_height = inclusion_proof.location().block_num(); + } + } else { + nullifier_requests.insert(note.nullifier()); + if inclusion_proof.location().block_num() < lowest_block_height { + lowest_block_height = inclusion_proof.location().block_num(); + } } - .into(), - )); + } - if let Some(block_height) = self + let nullifier_commit_heights = self .rpc_api - .get_nullifier_commit_height( - ¬e_record.nullifier(), - inclusion_proof.location().block_num(), - ) - .await? - { - if note_record.consumed_externally(note_record.nullifier(), block_height)? { - return Ok(Some(note_record)); - } + .get_nullifier_commit_heights(nullifier_requests, lowest_block_height) + .await?; - Ok(None) - } else { - let block_height = inclusion_proof.location().block_num(); - let current_block_num = self.get_sync_height().await?; + for (previous_note, note, inclusion_proof) in requested_notes { + let metadata = *note.metadata(); + let mut note_record = previous_note.unwrap_or(InputNoteRecord::new( + note.into(), + self.store.get_current_timestamp(), + ExpectedNoteState { + metadata: Some(metadata), + after_block_num: inclusion_proof.location().block_num(), + tag: Some(metadata.tag()), + } + .into(), + )); + + if let Some(Some(block_height)) = nullifier_commit_heights.get(¬e_record.nullifier()) + { + if note_record.consumed_externally(note_record.nullifier(), *block_height)? { + note_records.push(Some(note_record)); + } + + note_records.push(None); + } else { + let block_height = inclusion_proof.location().block_num(); + let current_block_num = self.get_sync_height().await?; + + let mut note_changed = + note_record.inclusion_proof_received(inclusion_proof, metadata)?; - let mut note_changed = - note_record.inclusion_proof_received(inclusion_proof, metadata)?; + if block_height <= current_block_num { + // FIXME: We should be able to build the mmr only once (outside the for loop). + // For some reason this leads to error, probably related to: + // https://github.com/0xMiden/miden-client/issues/1205 + // If the note is committed in the past we need to manually fetch the block + // header and MMR proof to verify the inclusion proof. + let mut current_partial_mmr = self.store.get_current_partial_mmr().await?; - if block_height <= current_block_num { - // If the note is committed in the past we need to manually fetch the block - // header and MMR proof to verify the inclusion proof. - let mut current_partial_mmr = self.store.get_current_partial_mmr().await?; + let block_header = self + .get_and_store_authenticated_block(block_height, &mut current_partial_mmr) + .await?; - let block_header = self - .get_and_store_authenticated_block(block_height, &mut current_partial_mmr) + note_changed |= note_record.block_header_received(&block_header)?; + } else { + // If the note is in the future we import it as unverified. We add the note tag + // so that the note is verified naturally in the next sync. + self.insert_note_tag(NoteTagRecord::with_note_source( + metadata.tag(), + note_record.id(), + )) .await?; + } - note_changed |= note_record.block_header_received(&block_header)?; - } else { - // If the note is in the future we import it as unverified. We add the note tag so - // that the note is verified naturally in the next sync. - self.insert_note_tag(NoteTagRecord::with_note_source( - metadata.tag(), - note_record.id(), - )) - .await?; + if note_changed { + note_records.push(Some(note_record)); + } else { + note_records.push(None); + } } - - if note_changed { Ok(Some(note_record)) } else { Ok(None) } } + + Ok(note_records) } - /// Builds a note record from the note details. If a note with the same ID was already stored it - /// is passed via `previous_note` so it can be updated. - async fn import_note_record_by_details( + /// Builds a note record list from note details. If a note with the same ID was already stored + /// it is passed via `previous_note` so it can be updated. + async fn import_note_records_by_details( &mut self, - previous_note: Option, - details: NoteDetails, - after_block_num: BlockNumber, - tag: Option, - ) -> Result, ClientError> { - let mut note_record = previous_note.unwrap_or({ - InputNoteRecord::new( - details, - self.store.get_current_timestamp(), - ExpectedNoteState { metadata: None, after_block_num, tag }.into(), - ) - }); - - let committed_note_data = if let Some(tag) = tag { - self.check_expected_note(after_block_num, tag, note_record.details()).await? - } else { - None - }; - - match committed_note_data { - Some((metadata, inclusion_proof)) => { - let mut current_partial_mmr = self.store.get_current_partial_mmr().await?; - let block_header = self - .get_and_store_authenticated_block( - inclusion_proof.location().block_num(), - &mut current_partial_mmr, - ) - .await?; - - let note_changed = - note_record.inclusion_proof_received(inclusion_proof, metadata)?; - - if note_record.block_header_received(&block_header)? | note_changed { - self.store - .remove_note_tag(NoteTagRecord::with_note_source( - metadata.tag(), - note_record.id(), - )) + requested_notes: Vec<(Option, NoteDetails, BlockNumber, Option)>, + ) -> Result>, ClientError> { + let mut lowest_request_block: BlockNumber = u32::MAX.into(); + let mut note_requests = vec![]; + for (_, details, after_block_num, tag) in &requested_notes { + if let Some(tag) = tag { + note_requests.push((details.id(), tag)); + if after_block_num < &lowest_request_block { + lowest_request_block = *after_block_num; + } + } + } + let mut committed_notes_data = + self.check_expected_notes(lowest_request_block, note_requests).await?; + + let mut note_records = vec![]; + for (previous_note, details, after_block_num, tag) in requested_notes { + let mut note_record = previous_note.unwrap_or({ + InputNoteRecord::new( + details, + self.store.get_current_timestamp(), + ExpectedNoteState { metadata: None, after_block_num, tag }.into(), + ) + }); + + match committed_notes_data.remove(¬e_record.id()) { + Some(Some((metadata, inclusion_proof))) => { + // FIXME: We should be able to build the mmr only once (outside the for loop). + // For some reason this leads to error, probably related to: + // https://github.com/0xMiden/miden-client/issues/1205 + let mut current_partial_mmr = self.store.get_current_partial_mmr().await?; + let block_header = self + .get_and_store_authenticated_block( + inclusion_proof.location().block_num(), + &mut current_partial_mmr, + ) .await?; - Ok(Some(note_record)) - } else { - Ok(None) - } - }, - None => Ok(Some(note_record)), + let note_changed = + note_record.inclusion_proof_received(inclusion_proof, metadata)?; + + if note_record.block_header_received(&block_header)? | note_changed { + self.store + .remove_note_tag(NoteTagRecord::with_note_source( + metadata.tag(), + note_record.id(), + )) + .await?; + + note_records.push(Some(note_record)); + } else { + note_records.push(None); + } + }, + _ => { + note_records.push(Some(note_record)); + }, + } } + + Ok(note_records) } - /// Checks if a note with the given `note_tag` and ID is present in the chain between the - /// `request_block_num` and the current block. If found it returns its metadata and inclusion + /// Checks if notes with their given `note_tag` and ID are present in the chain between the + /// `request_block_num` and the current block. If found it returns their metadata and inclusion /// proof. - async fn check_expected_note( + async fn check_expected_notes( &mut self, mut request_block_num: BlockNumber, - tag: NoteTag, - expected_note: &NoteDetails, - ) -> Result, ClientError> { + // Expected notes with their tags + expected_notes: Vec<(NoteId, &NoteTag)>, + ) -> Result>, ClientError> { + let tracked_tags: BTreeSet = expected_notes.iter().map(|(_, tag)| **tag).collect(); + let mut retrieved_proofs = BTreeMap::new(); let current_block_num = self.get_sync_height().await?; loop { if request_block_num > current_block_num { - return Ok(None); + break; } - let sync_notes = self - .rpc_api - .sync_notes(request_block_num, None, &[tag].into_iter().collect()) - .await?; + let sync_notes = + self.rpc_api.sync_notes(request_block_num, None, &tracked_tags).await?; - // This means that notes with that note_tag were found. - // Therefore, we should check if a note with the same id was found. - let committed_note = - sync_notes.notes.iter().find(|note| note.note_id() == &expected_note.id()); + for sync_note in sync_notes.notes { + if !expected_notes.iter().any(|(id, _)| id == sync_note.note_id()) { + continue; + } - if let Some(note) = committed_note { // This means that a note with the same id was found. // Therefore, we should mark the note as committed. let note_block_num = sync_notes.block_header.block_num(); if note_block_num > current_block_num { - return Ok(None); + break; } let note_inclusion_proof = NoteInclusionProof::new( note_block_num, - note.note_index(), - note.inclusion_path().clone(), + sync_note.note_index(), + sync_note.inclusion_path().clone(), )?; - return Ok(Some((note.metadata(), note_inclusion_proof))); + retrieved_proofs.insert( + *sync_note.note_id(), + Some((sync_note.metadata(), note_inclusion_proof)), + ); } - // We might have reached the chain tip without having found the note, bail if so + // We might have reached the chain tip without having found some notes, bail if so if sync_notes.block_header.block_num() == sync_notes.chain_tip { - return Ok(None); + break; } // This means that a note with the same id was not found. @@ -313,5 +426,6 @@ where // (sync_notes.block_header.unwrap()). request_block_num = sync_notes.block_header.block_num(); } + Ok(retrieved_proofs) } } diff --git a/crates/rust-client/src/note_transport/mod.rs b/crates/rust-client/src/note_transport/mod.rs index 2b9870c2c..6fd8f066e 100644 --- a/crates/rust-client/src/note_transport/mod.rs +++ b/crates/rust-client/src/note_transport/mod.rs @@ -128,6 +128,7 @@ where let sync_height = self.get_sync_height().await?; // Import fetched notes + let mut note_requests = Vec::with_capacity(notes.len()); for note in notes { let tag = note.metadata().tag(); let note_file = NoteFile::NoteDetails { @@ -135,8 +136,9 @@ where after_block_num: sync_height, tag: Some(tag), }; - self.import_note(note_file).await?; + note_requests.push(note_file); } + self.import_notes(¬e_requests).await?; // Update cursor (pagination) self.store.update_note_transport_cursor(rcursor).await?; diff --git a/crates/rust-client/src/rpc/mod.rs b/crates/rust-client/src/rpc/mod.rs index e3c37f1ca..cf785be8d 100644 --- a/crates/rust-client/src/rpc/mod.rs +++ b/crates/rust-client/src/rpc/mod.rs @@ -41,7 +41,7 @@ //! [`NodeRpcClient`] trait. use alloc::boxed::Box; -use alloc::collections::BTreeSet; +use alloc::collections::{BTreeMap, BTreeSet}; use alloc::string::String; use alloc::vec::Vec; use core::fmt; @@ -233,17 +233,27 @@ pub trait NodeRpcClient: Send + Sync { /// /// The default implementation of this method uses /// [`NodeRpcClient::sync_nullifiers`]. - async fn get_nullifier_commit_height( + async fn get_nullifier_commit_heights( &self, - nullifier: &Nullifier, - block_num: BlockNumber, - ) -> Result, RpcError> { - let nullifiers = self.sync_nullifiers(&[nullifier.prefix()], block_num, None).await?; + requested_nullifiers: BTreeSet, + block_from: BlockNumber, + ) -> Result>, RpcError> { + let prefixes: Vec = + requested_nullifiers.iter().map(crate::note::Nullifier::prefix).collect(); + let retrieved_nullifiers = self.sync_nullifiers(&prefixes, block_from, None).await?; + + let mut nullifiers_height = BTreeMap::new(); + for nullifier in requested_nullifiers { + if let Some(update) = + retrieved_nullifiers.iter().find(|update| update.nullifier == nullifier) + { + nullifiers_height.insert(nullifier, Some(update.block_num)); + } else { + nullifiers_height.insert(nullifier, None); + } + } - Ok(nullifiers - .iter() - .find(|update| update.nullifier == *nullifier) - .map(|update| update.block_num)) + Ok(nullifiers_height) } /// Fetches public note-related data for a list of [`NoteId`] and builds [`InputNoteRecord`]s diff --git a/crates/testing/miden-client-tests/src/tests.rs b/crates/testing/miden-client-tests/src/tests.rs index bad62b0fe..bb6afa9ab 100644 --- a/crates/testing/miden-client-tests/src/tests.rs +++ b/crates/testing/miden-client-tests/src/tests.rs @@ -140,10 +140,10 @@ async fn input_notes_round_trip() { // insert notes into database for note in &available_notes { client - .import_note(NoteFile::NoteWithProof( + .import_notes(&[NoteFile::NoteWithProof( note.note().unwrap().clone(), note.inclusion_proof().clone(), - )) + )]) .await .unwrap(); } @@ -174,11 +174,11 @@ async fn get_input_note() { // insert Note into database let note: InputNoteRecord = original_note.clone().into(); client - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.into(), tag: None, after_block_num: 0.into(), - }) + }]) .await .unwrap(); @@ -358,11 +358,11 @@ async fn sync_state() { for note in &expected_notes { client - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.clone().into(), after_block_num: 0.into(), tag: Some(note.metadata().tag()), - }) + }]) .await .unwrap(); } @@ -409,11 +409,11 @@ async fn sync_state_mmr() { for note in ¬es { client - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: note.clone().into(), after_block_num: 0.into(), tag: Some(note.metadata().tag()), - }) + }]) .await .unwrap(); } @@ -574,14 +574,15 @@ async fn import_note_validation() { for note in &available_notes { let Some(public_note) = note.note() else { continue }; - let nullifier_consumed = rpc_api - .get_nullifier_commit_height( - &public_note.nullifier(), + let nullifiers = rpc_api + .get_nullifier_commit_heights( + BTreeSet::from([public_note.nullifier()]), note.inclusion_proof().location().block_num(), ) .await .unwrap(); + let nullifier_consumed = nullifiers.get(&public_note.nullifier()).unwrap(); if nullifier_consumed.is_some() { consumed_note = Some(note.clone()); } else if expected_note.is_none() { @@ -597,19 +598,19 @@ async fn import_note_validation() { let consumed_note = consumed_note.expect("expected to find at least one consumed note"); client - .import_note(NoteFile::NoteWithProof( + .import_notes(&[NoteFile::NoteWithProof( consumed_note.note().unwrap().clone(), consumed_note.inclusion_proof().clone(), - )) + )]) .await .unwrap(); client - .import_note(NoteFile::NoteDetails { + .import_notes(&[NoteFile::NoteDetails { details: expected_note.note().unwrap().into(), after_block_num: 0.into(), tag: None, - }) + }]) .await .unwrap(); @@ -703,7 +704,7 @@ async fn import_processing_note_returns_error() { assert!(matches!( client - .import_note(NoteFile::NoteId(processing_notes[0].id())) + .import_notes(&[NoteFile::NoteId(processing_notes[0].id())]) .await .unwrap_err(), ClientError::NoteImportError { .. } @@ -2120,25 +2121,25 @@ const BUMP_MAP_CODE: &str = r#" pub proc bump_map_item # map key push.{map_key} - + # push slot_id_prefix, slot_id_suffix for the map slot push.MAP_SLOT[0..2] exec.::miden::protocol::active_account::get_map_item add.1 push.{map_key} - + # push slot_id_prefix, slot_id_suffix for the map slot push.MAP_SLOT[0..2] exec.::miden::protocol::native_account::set_map_item dropw # => [OLD_VALUE] - + dupw - + # push slot_id_prefix, slot_id_suffix for the map slot push.MAP_SLOT[0..2] - + # Set a new item each time as the value keeps changing exec.::miden::protocol::native_account::set_map_item dropw dropw diff --git a/crates/web-client/src/import.rs b/crates/web-client/src/import.rs index fccac22ca..99f87a6bd 100644 --- a/crates/web-client/src/import.rs +++ b/crates/web-client/src/import.rs @@ -92,9 +92,9 @@ impl WebClient { pub async fn import_note_file(&mut self, note_file: NoteFile) -> Result { if let Some(client) = self.get_mut_inner() { Ok(client - .import_note(note_file.into()) + .import_notes(&[note_file.into()]) .await - .map_err(|err| js_error_with_context(err, "failed to import note"))? + .map_err(|err| js_error_with_context(err, "failed to import note"))?[0] .into()) } else { Err(JsValue::from_str("Client not initialized"))