diff --git a/CHANGELOG.md b/CHANGELOG.md index ecf95062c..b2f8023ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +* Added a mechanism to retrieve MMR data whenever a note created on a past block is imported. +* Changed the way notes are added to the database based on `ExecutedTransaction`. * Added more feedback information to commands `info`, `notes list`, `notes show`, `account new`, `notes import`, `tx new` and `sync`. * Add `consumer_account_id` to `InputNoteRecord` with an implementation for sqlite store. * Renamed the cli `input-notes` command to `notes`. Now we only export notes that were created on this client as the result of a transaction. diff --git a/src/client/mod.rs b/src/client/mod.rs index f4b0de926..7254eaf00 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -19,7 +19,7 @@ mod chain_data; mod note_screener; mod notes; pub mod store_authenticator; -pub(crate) mod sync; +pub mod sync; pub mod transactions; pub use note_screener::NoteRelevance; pub(crate) use note_screener::NoteScreener; diff --git a/src/client/notes.rs b/src/client/notes.rs index eebd8bd21..4a78c68c3 100644 --- a/src/client/notes.rs +++ b/src/client/notes.rs @@ -5,12 +5,9 @@ use miden_objects::{ notes::{NoteId, NoteInclusionProof, NoteScript}, }; use miden_tx::{ScriptTarget, TransactionAuthenticator}; +use tracing::info; -use super::{ - note_screener::NoteRelevance, - rpc::{NodeRpcClient, NoteDetails}, - Client, -}; +use super::{note_screener::NoteRelevance, rpc::NodeRpcClient, Client}; use crate::{ client::NoteScreener, errors::ClientError, @@ -105,11 +102,11 @@ impl Client /// not the method verifies the existence of the note in the chain. /// /// If the imported note is verified to be on chain and it doesn't contain an inclusion proof - /// the method tries to build one if possible. + /// the method tries to build one. /// If the verification fails then a [ClientError::ExistenceVerificationError] is raised. pub async fn import_input_note( &mut self, - mut note: InputNoteRecord, + note: InputNoteRecord, verify: bool, ) -> Result<(), ClientError> { if !verify { @@ -124,44 +121,50 @@ impl Client } let note_details = chain_notes.pop().expect("chain_notes should have at least one element"); + let inclusion_details = note_details.inclusion_details(); - let inclusion_details = match note_details { - NoteDetails::OffChain(_, _, inclusion) => inclusion, - NoteDetails::Public(_, inclusion) => inclusion, - }; - - // Check to see if it's possible to create an inclusion proof if the note doesn't have one. - // Only do this if the note exists in the chain and the client is synced to a height equal or - // greater than the note's creation block. - if note.inclusion_proof().is_none() - && self.get_sync_height()? >= inclusion_details.block_num - { + // If the note exists in the chain and the client is synced to a height equal or + // greater than the note's creation block, get MMR and block header data for the + // note's block. Additionally create the inclusion proof if none is provided. + let inclusion_proof = if self.get_sync_height()? >= inclusion_details.block_num { // Add the inclusion proof to the imported note - let block_header = self - .rpc_api - .get_block_header_by_number(Some(inclusion_details.block_num), false) - .await?; + info!("Requesting MMR data for past block num {}", inclusion_details.block_num); + let block_header = + self.get_and_store_authenticated_block(inclusion_details.block_num).await?; - let inclusion_proof = NoteInclusionProof::new( + let built_inclusion_proof = NoteInclusionProof::new( inclusion_details.block_num, block_header.sub_hash(), block_header.note_root(), inclusion_details.note_index.into(), - inclusion_details.merkle_path, + inclusion_details.merkle_path.clone(), )?; - note = InputNoteRecord::new( - note.id(), - note.recipient(), - note.assets().clone(), - note.status(), - note.metadata().copied(), - Some(inclusion_proof), - note.details().clone(), - None, - ); - } + // If the imported note already provides an inclusion proof, check that + // it equals the one we constructed from node data. + if let Some(proof) = note.inclusion_proof() { + if proof != &built_inclusion_proof { + return Err(ClientError::NoteImportError( + "Constructed inclusion proof does not equal the provided one".to_string(), + )); + } + } + + Some(built_inclusion_proof) + } else { + None + }; + let note = InputNoteRecord::new( + note.id(), + note.recipient(), + note.assets().clone(), + note.status(), + note.metadata().copied(), + inclusion_proof, + note.details().clone(), + None, + ); self.store.insert_input_note(¬e).map_err(|err| err.into()) } diff --git a/src/client/rpc/mod.rs b/src/client/rpc/mod.rs index 71d8a713d..b7e64c1bd 100644 --- a/src/client/rpc/mod.rs +++ b/src/client/rpc/mod.rs @@ -3,7 +3,7 @@ use core::fmt; use async_trait::async_trait; use miden_objects::{ accounts::{Account, AccountId}, - crypto::merkle::{MerklePath, MmrDelta}, + crypto::merkle::{MerklePath, MmrDelta, MmrProof}, notes::{Note, NoteId, NoteMetadata, NoteTag}, transaction::ProvenTransaction, BlockHeader, Digest, @@ -23,6 +23,15 @@ pub enum NoteDetails { Public(Note, NoteInclusionDetails), } +impl NoteDetails { + pub fn inclusion_details(&self) -> &NoteInclusionDetails { + match self { + NoteDetails::OffChain(_, _, inclusion_details) => inclusion_details, + NoteDetails::Public(_, inclusion_details) => inclusion_details, + } + } +} + /// Describes the possible responses from the `GetAccountDetails` endpoint for an account pub enum AccountDetails { OffChain(AccountId, AccountUpdateSummary), @@ -75,14 +84,16 @@ pub trait NodeRpcClient { ) -> Result<(), NodeRpcClientError>; /// Given a block number, fetches the block header corresponding to that height from the node - /// using the `/GetBlockHeaderByNumber` endpoint + /// using the `/GetBlockHeaderByNumber` endpoint. + /// If `include_mmr_proof` is set to true and the function returns an `Ok`, the second value + /// of the return tuple should always be Some(MmrProof) /// /// When `None` is provided, returns info regarding the latest block async fn get_block_header_by_number( &mut self, - block_number: Option, + block_num: Option, include_mmr_proof: bool, - ) -> Result; + ) -> Result<(BlockHeader, Option), NodeRpcClientError>; /// Fetches note-related data for a list of [NoteId] using the `/GetNotesById` rpc endpoint /// diff --git a/src/client/rpc/tonic_client.rs b/src/client/rpc/tonic_client.rs index 1fff4df7b..41bfa5024 100644 --- a/src/client/rpc/tonic_client.rs +++ b/src/client/rpc/tonic_client.rs @@ -14,6 +14,7 @@ use miden_node_proto::{ }; use miden_objects::{ accounts::{Account, AccountId}, + crypto::merkle::{MerklePath, MmrProof}, notes::{Note, NoteId, NoteMetadata, NoteTag, NoteType}, transaction::ProvenTransaction, utils::Deserializable, @@ -91,11 +92,12 @@ impl NodeRpcClient for TonicRpcClient { &mut self, block_num: Option, include_mmr_proof: bool, - ) -> Result { + ) -> Result<(BlockHeader, Option), NodeRpcClientError> { let request = GetBlockHeaderByNumberRequest { block_num, include_mmr_proof: Some(include_mmr_proof), }; + let rpc_api = self.rpc_api().await?; let api_response = rpc_api.get_block_header_by_number(request).await.map_err(|err| { NodeRpcClientError::RequestError( @@ -104,12 +106,38 @@ impl NodeRpcClient for TonicRpcClient { ) })?; - api_response - .into_inner() + let response = api_response.into_inner(); + + let block_header: BlockHeader = response .block_header .ok_or(NodeRpcClientError::ExpectedFieldMissing("BlockHeader".into()))? .try_into() - .map_err(|err: ConversionError| NodeRpcClientError::ConversionFailure(err.to_string())) + .map_err(|err: ConversionError| { + NodeRpcClientError::ConversionFailure(err.to_string()) + })?; + + let mmr_proof = if include_mmr_proof { + let forest = response + .chain_length + .ok_or(NodeRpcClientError::ExpectedFieldMissing("ChainLength".into()))?; + let merkle_path: MerklePath = response + .mmr_path + .ok_or(NodeRpcClientError::ExpectedFieldMissing("MmrPath".into()))? + .try_into() + .map_err(|err: ConversionError| { + NodeRpcClientError::ConversionFailure(err.to_string()) + })?; + + Some(MmrProof { + forest: forest as usize, + position: block_header.block_num() as usize, + merkle_path, + }) + } else { + None + }; + + Ok((block_header, mmr_proof)) } async fn get_notes_by_id( diff --git a/src/client/sync.rs b/src/client/sync.rs index 12c523d71..567526533 100644 --- a/src/client/sync.rs +++ b/src/client/sync.rs @@ -4,7 +4,7 @@ use std::cmp::max; use crypto::merkle::{InOrderIndex, MmrDelta, MmrPeaks, PartialMmr}; use miden_objects::{ accounts::{Account, AccountId, AccountStub}, - crypto::{self, rand::FeltRng}, + crypto::{self, merkle::MerklePath, rand::FeltRng}, notes::{Note, NoteId, NoteInclusionProof, NoteInputs, NoteRecipient, NoteTag}, transaction::{InputNote, TransactionId}, BlockHeader, Digest, @@ -230,7 +230,7 @@ impl Client /// Calls `get_block_header_by_number` requesting the genesis block and storing it /// in the local database async fn retrieve_and_store_genesis(&mut self) -> Result<(), ClientError> { - let genesis_block = self.rpc_api.get_block_header_by_number(Some(0), false).await?; + let (genesis_block, _) = self.rpc_api.get_block_header_by_number(Some(0), false).await?; let blank_mmr_peaks = MmrPeaks::new(0, vec![]).expect("Blank MmrPeaks should not fail to instantiate"); @@ -640,6 +640,54 @@ impl Client } Ok(()) } + + /// Retrieves and stores a [BlockHeader] by number, and stores its authentication data as well. + pub(crate) async fn get_and_store_authenticated_block( + &mut self, + block_num: u32, + ) -> Result { + let mut current_partial_mmr = self.build_current_partial_mmr()?; + + if current_partial_mmr.is_tracked(block_num as usize) { + warn!("Current partial MMR already contains the requested data"); + let (block_header, _) = self.store().get_block_header_by_num(block_num)?; + return Ok(block_header); + } + + let (block_header, mmr_proof) = + self.rpc_api.get_block_header_by_number(Some(block_num), true).await?; + + let mut path_nodes: Vec<(InOrderIndex, Digest)> = vec![]; + + let mmr_proof = mmr_proof + .expect("NodeRpcApi::get_block_header_by_number() should have returned an MMR proof"); + // Trim merkle path to keep nodes relevant to our current PartialMmr + let rightmost_index = InOrderIndex::from_leaf_pos(current_partial_mmr.forest() - 1); + let mut idx = InOrderIndex::from_leaf_pos(block_num as usize); + for node in mmr_proof.merkle_path { + idx = idx.sibling(); + // Rightmost index is always the biggest value, so if the path contains any node + // past it, we can discard it for our version of the forest + if idx > rightmost_index { + continue; + } + path_nodes.push((idx, node)); + idx = idx.parent(); + } + + let merkle_path = MerklePath::new(path_nodes.iter().map(|(_, n)| *n).collect()); + + current_partial_mmr + .track(block_num as usize, block_header.hash(), &merkle_path) + .map_err(StoreError::MmrError)?; + + // Insert header and MMR nodes + self.store + .insert_block_header(block_header, current_partial_mmr.peaks(), true)?; + self.store.insert_chain_mmr_nodes(&path_nodes)?; + + Ok(block_header) + } } // UTILS diff --git a/src/errors.rs b/src/errors.rs index 30753227f..371a6a69f 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -22,6 +22,7 @@ pub enum ClientError { ImportNewAccountWithoutSeed, MissingOutputNotes(Vec), NoteError(NoteError), + NoteImportError(String), NoteRecordError(String), NoConsumableNoteForAccount(AccountId), NodeRpcClientError(NodeRpcClientError), @@ -56,6 +57,7 @@ impl fmt::Display for ClientError { write!(f, "No consumable note for account ID {}", account_id) }, ClientError::NoteError(err) => write!(f, "note error: {err}"), + ClientError::NoteImportError(err) => write!(f, "error importing note: {err}"), ClientError::NoteRecordError(err) => write!(f, "note record error: {err}"), ClientError::NodeRpcClientError(err) => write!(f, "rpc api error: {err}"), ClientError::ScreenerError(err) => write!(f, "note screener error: {err}"), diff --git a/src/mock.rs b/src/mock.rs index 604671b43..4d02b72c8 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -20,7 +20,7 @@ use miden_objects::{ assets::{Asset, AssetVault, FungibleAsset, TokenSymbol}, crypto::{ dsa::rpo_falcon512::SecretKey, - merkle::{Mmr, MmrDelta, NodeIndex, SimpleSmt}, + merkle::{Mmr, MmrDelta, MmrProof, NodeIndex, SimpleSmt}, rand::RpoRandomCoin, }, notes::{ @@ -130,9 +130,9 @@ impl NodeRpcClient for MockRpcApi { &mut self, block_num: Option, _include_mmr_proof: bool, - ) -> Result { + ) -> Result<(BlockHeader, Option), NodeRpcClientError> { if block_num == Some(0) { - return Ok(self.genesis_block); + return Ok((self.genesis_block, None)); } panic!("get_block_header_by_number is supposed to be only used for genesis block") } diff --git a/src/store/mod.rs b/src/store/mod.rs index c7257560c..011c7a8a7 100644 --- a/src/store/mod.rs +++ b/src/store/mod.rs @@ -129,6 +129,11 @@ pub trait Store { filter: ChainMmrNodeFilter, ) -> Result, StoreError>; + /// Inserts MMR authentication nodes. + /// + /// In the case where the [InOrderIndex] already exists on the table, the insertion is ignored + fn insert_chain_mmr_nodes(&self, nodes: &[(InOrderIndex, Digest)]) -> Result<(), StoreError>; + /// Returns peaks information from the blockchain by a specific block number. /// /// If there is no chain MMR info stored for the provided block returns an empty [MmrPeaks] diff --git a/src/store/sqlite_store/chain_data.rs b/src/store/sqlite_store/chain_data.rs index 185b1ea90..7430d320c 100644 --- a/src/store/sqlite_store/chain_data.rs +++ b/src/store/sqlite_store/chain_data.rs @@ -39,17 +39,12 @@ impl SqliteStore { chain_mmr_peaks: MmrPeaks, has_client_notes: bool, ) -> Result<(), StoreError> { - let chain_mmr_peaks = chain_mmr_peaks.peaks().to_vec(); - let (block_num, header, chain_mmr, has_client_notes) = - serialize_block_header(block_header, chain_mmr_peaks, has_client_notes)?; - const QUERY: &str = "\ - INSERT INTO block_headers - (block_num, header, chain_mmr_peaks, has_client_notes) - VALUES (?, ?, ?, ?)"; + let mut db = self.db(); + let tx = db.transaction()?; - self.db() - .execute(QUERY, params![block_num, header, chain_mmr, has_client_notes])?; + Self::insert_block_header_tx(&tx, block_header, chain_mmr_peaks, has_client_notes)?; + tx.commit()?; Ok(()) } @@ -123,8 +118,20 @@ impl SqliteStore { Ok(MmrPeaks::new(0, vec![])?) } + pub fn insert_chain_mmr_nodes( + &self, + nodes: &[(InOrderIndex, Digest)], + ) -> Result<(), StoreError> { + let mut db = self.db(); + let tx = db.transaction()?; + + Self::insert_chain_mmr_nodes_tx(&tx, nodes)?; + + Ok(tx.commit().map(|_| ())?) + } + /// Inserts a list of MMR authentication nodes to the Chain MMR nodes table. - pub(crate) fn insert_chain_mmr_nodes( + pub(crate) fn insert_chain_mmr_nodes_tx( tx: &Transaction<'_>, nodes: &[(InOrderIndex, Digest)], ) -> Result<(), StoreError> { @@ -145,7 +152,7 @@ impl SqliteStore { let (block_num, header, chain_mmr, has_client_notes) = serialize_block_header(block_header, chain_mmr_peaks, has_client_notes)?; const QUERY: &str = "\ - INSERT INTO block_headers + INSERT OR IGNORE INTO block_headers (block_num, header, chain_mmr_peaks, has_client_notes) VALUES (?, ?, ?, ?)"; tx.execute(QUERY, params![block_num, header, chain_mmr, has_client_notes])?; @@ -163,7 +170,7 @@ fn insert_chain_mmr_node( node: Digest, ) -> Result<(), StoreError> { let (id, node) = serialize_chain_mmr_node(id, node)?; - const QUERY: &str = "INSERT INTO chain_mmr_nodes (id, node) VALUES (?, ?)"; + const QUERY: &str = "INSERT OR IGNORE INTO chain_mmr_nodes (id, node) VALUES (?, ?)"; tx.execute(QUERY, params![id, node])?; Ok(()) } diff --git a/src/store/sqlite_store/mod.rs b/src/store/sqlite_store/mod.rs index c243ae0d4..9794ce28f 100644 --- a/src/store/sqlite_store/mod.rs +++ b/src/store/sqlite_store/mod.rs @@ -190,6 +190,10 @@ impl Store for SqliteStore { self.get_chain_mmr_nodes(filter) } + fn insert_chain_mmr_nodes(&self, nodes: &[(InOrderIndex, Digest)]) -> Result<(), StoreError> { + self.insert_chain_mmr_nodes(nodes) + } + fn get_chain_mmr_peaks_by_block_num(&self, block_num: u32) -> Result { self.get_chain_mmr_peaks_by_block_num(block_num) } diff --git a/src/store/sqlite_store/sync.rs b/src/store/sqlite_store/sync.rs index 496e16b8f..174da825c 100644 --- a/src/store/sqlite_store/sync.rs +++ b/src/store/sqlite_store/sync.rs @@ -105,7 +105,7 @@ impl SqliteStore { 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, &new_authentication_nodes)?; + Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?; // Update tracked output notes for (note_id, inclusion_proof) in committed_notes.updated_output_notes().iter() { diff --git a/tests/integration/common.rs b/tests/integration/common.rs index 82892150f..21c3072f1 100644 --- a/tests/integration/common.rs +++ b/tests/integration/common.rs @@ -10,6 +10,7 @@ use miden_client::{ get_random_coin, rpc::TonicRpcClient, store_authenticator::StoreAuthenticator, + sync::SyncSummary, transactions::transaction_request::{TransactionRequest, TransactionTemplate}, Client, }, @@ -105,6 +106,24 @@ pub async fn execute_tx_and_sync(client: &mut TestClient, tx_request: Transactio } } +// Syncs until `amount_of_blocks` have been created onchain compared to client's sync height +pub async fn wait_for_blocks(client: &mut TestClient, amount_of_blocks: u32) -> SyncSummary { + let current_block = client.get_sync_height().unwrap(); + let final_block = current_block + amount_of_blocks; + println!("Syncing until block {}...", final_block); + // wait until tx is committed + loop { + let summary = client.sync_state().await.unwrap(); + println!("Synced to block {} (syncing until {})...", summary.block_num, final_block); + + if summary.block_num >= final_block { + return summary; + } + + std::thread::sleep(std::time::Duration::new(3, 0)); + } +} + /// Waits for node to be running. /// /// # Panics diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 4c32d67df..4f3e33f58 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -372,7 +372,7 @@ async fn test_get_output_notes() { let faucet_account_id = faucet_account_stub.id(); let random_account_id = AccountId::from_hex("0x0123456789abcdef").unwrap(); - //No output notes initially + // No output notes initially assert!(client.get_output_notes(NoteFilter::All).unwrap().is_empty()); // First Mint necesary token @@ -398,12 +398,12 @@ async fn test_get_output_notes() { let output_note_id = tx_request.expected_output_notes()[0].id(); - //Before executing, the output note is not found + // Before executing, the output note is not found assert!(client.get_output_note(output_note_id).is_err()); execute_tx_and_sync(&mut client, tx_request).await; - //After executing, the note is only found in output notes + // After executing, the note is only found in output notes assert!(client.get_output_note(output_note_id).is_ok()); assert!(client.get_input_note(output_note_id).is_err()); } @@ -413,31 +413,45 @@ async fn test_import_pending_notes() { let mut client_1 = create_test_client(); let (first_basic_account, _second_basic_account, faucet_account) = setup(&mut client_1, AccountStorageMode::Local).await; + let mut client_2 = create_test_client(); + let (client_2_account, _seed) = client_2 + .new_account(AccountTemplate::BasicWallet { + mutable_code: true, + storage_mode: AccountStorageMode::Local, + }) + .unwrap(); + wait_for_node(&mut client_2).await; - client_1.sync_state().await.unwrap(); - client_2.sync_state().await.unwrap(); let tx_template = TransactionTemplate::MintFungibleAsset( FungibleAsset::new(faucet_account.id(), MINT_AMOUNT).unwrap(), - first_basic_account.id(), + client_2_account.id(), NoteType::OffChain, ); let tx_request = client_1.build_transaction_request(tx_template).unwrap(); let note = tx_request.expected_output_notes()[0].clone(); + client_2.sync_state().await.unwrap(); // If the verification is requested before execution then the import should fail assert!(client_2.import_input_note(note.clone().into(), true).await.is_err()); execute_tx_and_sync(&mut client_1, tx_request).await; - client_2.sync_state().await.unwrap(); + // Use client 1 to wait until a couple of blocks have passed + wait_for_blocks(&mut client_1, 3).await; + + let new_sync_data = client_2.sync_state().await.unwrap(); client_2.import_input_note(note.clone().into(), true).await.unwrap(); let input_note = client_2.get_input_note(note.id()).unwrap(); + assert!(new_sync_data.block_num > input_note.inclusion_proof().unwrap().origin().block_num + 1); // If imported after execution and syncing then the inclusion proof should be Some assert!(input_note.inclusion_proof().is_some()); + // If client 2 succesfully consumes the note, we confirm we have MMR and block header data + consume_notes(&mut client_2, client_2_account.id(), &[input_note.try_into().unwrap()]).await; + let tx_template = TransactionTemplate::MintFungibleAsset( FungibleAsset::new(faucet_account.id(), MINT_AMOUNT).unwrap(), first_basic_account.id(),