feat: Use incoming MMR proof to add block authentications for notes c…

…reated in past blocks

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.

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;

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<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> 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<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> 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(&note).map_err(|err| err.into())
     }
 

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<u32>,
+        block_num: Option<u32>,
         include_mmr_proof: bool,
-    ) -> Result<BlockHeader, NodeRpcClientError>;
+    ) -> Result<(BlockHeader, Option<MmrProof>), NodeRpcClientError>;
 
     /// Fetches note-related data for a list of [NoteId] using the `/GetNotesById` rpc endpoint
     ///

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<u32>,
         include_mmr_proof: bool,
-    ) -> Result<BlockHeader, NodeRpcClientError> {
+    ) -> Result<(BlockHeader, Option<MmrProof>), 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(

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<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> 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<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> 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<BlockHeader, ClientError> {
+        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

src/errors.rs

@@ -22,6 +22,7 @@ pub enum ClientError {
     ImportNewAccountWithoutSeed,
     MissingOutputNotes(Vec<NoteId>),
     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}"),

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<u32>,
         _include_mmr_proof: bool,
-    ) -> Result<BlockHeader, NodeRpcClientError> {
+    ) -> Result<(BlockHeader, Option<MmrProof>), 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")
     }

src/store/mod.rs

@@ -129,6 +129,11 @@ pub trait Store {
         filter: ChainMmrNodeFilter,
     ) -> Result<BTreeMap<InOrderIndex, Digest>, 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]