chore: Address review comments

crates/miden-objects/src/batch/proposed_batch.rs

@@ -8,7 +8,7 @@ use crate::{
     account::AccountId,
     batch::{BatchAccountUpdate, BatchId, BatchNoteTree},
     block::{BlockHeader, BlockNumber},
-    errors::BatchProposeError,
+    errors::ProposedBatchError,
     note::{NoteHeader, NoteId, NoteInclusionProof},
     transaction::{
         ChainMmr, InputNoteCommitment, InputNotes, OutputNote, ProvenTransaction, TransactionId,
@@ -34,7 +34,7 @@ pub struct ProposedBatch {
     /// The note inclusion proofs for unauthenticated notes that were consumed in the batch which
     /// can be authenticated.
     unauthenticated_note_proofs: BTreeMap<NoteId, NoteInclusionProof>,
-    /// The ID of the batch.
+    /// The ID of the batch, which is a cryptographic commitment to the transactions in the batch.
     id: BatchId,
     /// A map from account ID's updated in this batch to the aggregated update from all
     /// transaction's that touched the account.
@@ -113,34 +113,34 @@ impl ProposedBatch {
         block_header: BlockHeader,
         chain_mmr: ChainMmr,
         unauthenticated_note_proofs: BTreeMap<NoteId, NoteInclusionProof>,
-    ) -> Result<Self, BatchProposeError> {
+    ) -> Result<Self, ProposedBatchError> {
         // Check for empty or duplicate transactions.
         // --------------------------------------------------------------------------------------------
 
         if transactions.is_empty() {
-            return Err(BatchProposeError::EmptyTransactionBatch);
+            return Err(ProposedBatchError::EmptyTransactionBatch);
         }
 
         let mut transaction_set = BTreeSet::new();
         for tx in transactions.iter() {
             if !transaction_set.insert(tx.id()) {
-                return Err(BatchProposeError::DuplicateTransaction { transaction_id: tx.id() });
+                return Err(ProposedBatchError::DuplicateTransaction { transaction_id: tx.id() });
             }
         }
 
         // Verify block header and chain MMR match.
         // --------------------------------------------------------------------------------------------
 
         if chain_mmr.chain_length() != block_header.block_num() {
-            return Err(BatchProposeError::InconsistentChainLength {
+            return Err(ProposedBatchError::InconsistentChainLength {
                 expected: block_header.block_num(),
                 actual: chain_mmr.chain_length(),
             });
         }
 
         let hashed_peaks = chain_mmr.peaks().hash_peaks();
         if hashed_peaks != block_header.chain_root() {
-            return Err(BatchProposeError::InconsistentChainRoot {
+            return Err(ProposedBatchError::InconsistentChainRoot {
                 expected: block_header.chain_root(),
                 actual: hashed_peaks,
             });
@@ -159,7 +159,7 @@ impl ProposedBatch {
 
         for tx in transactions.iter() {
             if !block_references.contains(&tx.block_ref()) {
-                return Err(BatchProposeError::MissingTransactionBlockReference {
+                return Err(ProposedBatchError::MissingTransactionBlockReference {
                     block_reference: tx.block_ref(),
                     transaction_id: tx.id(),
                 });
@@ -184,7 +184,7 @@ impl ProposedBatch {
                     // This returns an error if the transactions are not correctly ordered, e.g. if
                     // B comes before A.
                     occupied.into_mut().merge_proven_tx(tx).map_err(|source| {
-                        BatchProposeError::AccountUpdateError {
+                        ProposedBatchError::AccountUpdateError {
                             account_id: tx.account_id(),
                             source,
                         }
@@ -198,7 +198,7 @@ impl ProposedBatch {
         }
 
         if account_updates.len() > MAX_ACCOUNTS_PER_BATCH {
-            return Err(BatchProposeError::TooManyAccountUpdates(account_updates.len()));
+            return Err(ProposedBatchError::TooManyAccountUpdates(account_updates.len()));
         }
 
         // Check for duplicates in input notes.
@@ -213,7 +213,7 @@ impl ProposedBatch {
             for note in tx.input_notes() {
                 let nullifier = note.nullifier();
                 if let Some(first_transaction_id) = input_note_map.insert(nullifier, tx.id()) {
-                    return Err(BatchProposeError::DuplicateInputNote {
+                    return Err(ProposedBatchError::DuplicateInputNote {
                         note_nullifier: nullifier,
                         first_transaction_id,
                         second_transaction_id: tx.id(),
@@ -253,7 +253,7 @@ impl ProposedBatch {
                             let note_block_header = chain_mmr
                                 .get_block(proof.location().block_num())
                                 .ok_or_else(|| {
-                                    BatchProposeError::UnauthenticatedInputNoteBlockNotInChainMmr {
+                                    ProposedBatchError::UnauthenticatedInputNoteBlockNotInChainMmr {
                                         block_number: proof.location().block_num(),
                                         note_id: input_note_header.id(),
                                     }
@@ -280,14 +280,14 @@ impl ProposedBatch {
         let output_notes = output_notes.into_notes();
 
         if input_notes.len() > MAX_INPUT_NOTES_PER_BATCH {
-            return Err(BatchProposeError::TooManyInputNotes(input_notes.len()));
+            return Err(ProposedBatchError::TooManyInputNotes(input_notes.len()));
         }
         // SAFETY: This is safe as we have checked for duplicates and the max number of input notes
         // in a batch.
         let input_notes = InputNotes::new_unchecked(input_notes);
 
         if output_notes.len() > MAX_OUTPUT_NOTES_PER_BATCH {
-            return Err(BatchProposeError::TooManyOutputNotes(output_notes.len()));
+            return Err(ProposedBatchError::TooManyOutputNotes(output_notes.len()));
         }
 
         // Build the output notes SMT.
@@ -429,14 +429,14 @@ impl BatchOutputNoteTracker {
     /// - any output note is created more than once (by the same or different transactions).
     fn new<'a>(
         txs: impl Iterator<Item = &'a ProvenTransaction>,
-    ) -> Result<Self, BatchProposeError> {
+    ) -> Result<Self, ProposedBatchError> {
         let mut output_notes = BTreeMap::new();
         for tx in txs {
             for note in tx.output_notes().iter() {
                 if let Some((first_transaction_id, _)) =
                     output_notes.insert(note.id(), (tx.id(), note.clone()))
                 {
-                    return Err(BatchProposeError::DuplicateOutputNote {
+                    return Err(ProposedBatchError::DuplicateOutputNote {
                         note_id: note.id(),
                         first_transaction_id,
                         second_transaction_id: tx.id(),
@@ -461,7 +461,7 @@ impl BatchOutputNoteTracker {
     pub fn remove_note(
         &mut self,
         input_note_header: &NoteHeader,
-    ) -> Result<bool, BatchProposeError> {
+    ) -> Result<bool, ProposedBatchError> {
         let id = input_note_header.id();
         if let Some((_, output_note)) = self.output_notes.remove(&id) {
             // Check if the notes with the same ID have differing hashes.
@@ -470,7 +470,7 @@ impl BatchOutputNoteTracker {
             let input_hash = input_note_header.hash();
             let output_hash = output_note.hash();
             if output_hash != input_hash {
-                return Err(BatchProposeError::NoteHashesMismatch { id, input_hash, output_hash });
+                return Err(ProposedBatchError::NoteHashesMismatch { id, input_hash, output_hash });
             }
 
             return Ok(true);
@@ -494,13 +494,13 @@ fn authenticate_unauthenticated_note(
     note_header: &NoteHeader,
     proof: &NoteInclusionProof,
     block_header: &BlockHeader,
-) -> Result<(), BatchProposeError> {
+) -> Result<(), ProposedBatchError> {
     let note_index = proof.location().node_index_in_block().into();
     let note_hash = note_header.hash();
     proof
         .note_path()
         .verify(note_index, note_hash, &block_header.note_root())
-        .map_err(|source| BatchProposeError::UnauthenticatedNoteAuthenticationFailed {
+        .map_err(|source| ProposedBatchError::UnauthenticatedNoteAuthenticationFailed {
             note_id: note_header.id(),
             block_num: proof.location().block_num(),
             source,

crates/miden-objects/src/errors.rs

@@ -452,7 +452,7 @@ pub enum ProvenTransactionError {
 // ================================================================================================
 
 #[derive(Debug, Error)]
-pub enum BatchProposeError {
+pub enum ProposedBatchError {
     #[error(
         "transaction batch has {0} input notes but at most {MAX_INPUT_NOTES_PER_BATCH} are allowed"
     )]

crates/miden-objects/src/lib.rs

@@ -25,7 +25,7 @@ mod errors;
 pub use constants::*;
 pub use errors::{
     AccountDeltaError, AccountError, AccountIdError, AssetError, AssetVaultError,
-    BatchAccountUpdateError, BatchProposeError, BlockError, ChainMmrError, NoteError,
+    BatchAccountUpdateError, BlockError, ChainMmrError, NoteError, ProposedBatchError,
     ProvenTransactionError, TransactionInputError, TransactionOutputError, TransactionScriptError,
 };
 pub use miden_crypto::hash::rpo::{Rpo256 as Hasher, RpoDigest as Digest};

crates/miden-objects/src/testing/chain_mmr.rs

@@ -0,0 +1,46 @@
+use miden_crypto::merkle::{Mmr, PartialMmr};
+
+use crate::{block::BlockHeader, transaction::ChainMmr, ChainMmrError};
+
+impl ChainMmr {
+    /// Converts the [`Mmr`] into a [`ChainMmr`] by selectively copying all leaves that are in the
+    /// given `blocks` iterator.
+    ///
+    /// This tracks all blocks in the given iterator in the [`ChainMmr`] except for the block whose
+    /// block number equals [`Mmr::forest`], which is the current chain length.
+    ///
+    /// # Panics
+    ///
+    /// Due to being only available in test scenarios, this function panics when one of the given
+    /// blocks does not exist in the provided mmr.
+    pub fn from_mmr<I>(
+        mmr: &Mmr,
+        blocks: impl IntoIterator<Item = BlockHeader, IntoIter = I> + Clone,
+    ) -> Result<ChainMmr, ChainMmrError>
+    where
+        I: Iterator<Item = BlockHeader>,
+    {
+        // We do not include the latest block as it is used as the reference block and is added to
+        // the MMR by the transaction or batch kernel.
+
+        let target_forest = mmr.forest() - 1;
+        let peaks = mmr
+            .peaks_at(target_forest)
+            .expect("target_forest should be smaller than forest of the mmr");
+        let mut partial_mmr = PartialMmr::from_peaks(peaks);
+
+        for block_num in blocks
+            .clone()
+            .into_iter()
+            .map(|header| header.block_num().as_usize())
+            .filter(|block_num| *block_num < target_forest)
+        {
+            let leaf = mmr.get(block_num).expect("error: block num does not exist");
+            let path =
+                mmr.open_at(block_num, target_forest).expect("error: block proof").merkle_path;
+            partial_mmr.track(block_num, leaf, &path).expect("error: partial mmr track");
+        }
+
+        ChainMmr::new(partial_mmr, blocks)
+    }
+}

crates/miden-objects/src/testing/mod.rs

@@ -11,6 +11,7 @@ pub mod account_component;
 pub mod account_id;
 pub mod asset;
 pub mod block;
+pub mod chain_mmr;
 pub mod constants;
 pub mod note;
 pub mod storage;

crates/miden-objects/src/transaction/chain_mmr.rs

@@ -1,6 +1,5 @@
 use alloc::collections::BTreeMap;
 
-use miden_crypto::merkle::Mmr;
 use vm_core::utils::{Deserializable, Serializable};
 
 use crate::{
@@ -67,47 +66,6 @@ impl ChainMmr {
         Ok(Self { mmr, blocks: block_map })
     }
 
-    /// Converts the [`Mmr`] into a [`ChainMmr`] by selectively copying all leaves that are in the
-    /// given `blocks` iterator.
-    ///
-    /// This tracks all blocks in the given iterator in the [`ChainMmr`] except for the block whose
-    /// block number equals [`Mmr::forest`], which is the current chain length.
-    ///
-    /// # Panics
-    ///
-    /// Due to being only available in test scenarios, this function panics when one of the given
-    /// blocks does not exist in the provided mmr.
-    #[cfg(any(feature = "testing", test))]
-    pub fn from_mmr<I>(
-        mmr: &Mmr,
-        blocks: impl IntoIterator<Item = BlockHeader, IntoIter = I> + Clone,
-    ) -> Result<ChainMmr, ChainMmrError>
-    where
-        I: Iterator<Item = BlockHeader>,
-    {
-        // We do not include the latest block as it is used as the reference block and is added to
-        // the MMR by the transaction or batch kernel.
-        let target_forest = mmr.forest() - 1;
-        let peaks = mmr
-            .peaks_at(target_forest)
-            .expect("target_forest should be smaller than forest of the mmr");
-        let mut partial_mmr = PartialMmr::from_peaks(peaks);
-
-        for block_num in blocks
-            .clone()
-            .into_iter()
-            .map(|header| header.block_num().as_usize())
-            .filter(|block_num| *block_num < target_forest)
-        {
-            let leaf = mmr.get(block_num).expect("error: block num does not exist");
-            let path =
-                mmr.open_at(block_num, target_forest).expect("error: block proof").merkle_path;
-            partial_mmr.track(block_num, leaf, &path).expect("error: partial mmr track");
-        }
-
-        ChainMmr::new(partial_mmr, blocks)
-    }
-
     // PUBLIC ACCESSORS
     // --------------------------------------------------------------------------------------------
 

crates/miden-tx-batch-prover/Cargo.toml

@@ -26,7 +26,6 @@ std = [
 ]
 
 [dependencies]
-anyhow = { version = "1.0", default-features = false, optional = true }
 miden-crypto = { workspace = true }
 miden-tx = { workspace = true }
 miden-objects = { workspace = true }
@@ -35,8 +34,8 @@ vm-core = { workspace = true }
 vm-processor = { workspace = true }
 
 [dev-dependencies]
-rand = { workspace = true, features = ["small_rng"] }
-miden-lib = { workspace = true, features = ["std", "testing"] }
-winterfell = { version = "0.11" }
 anyhow = { version = "1.0", features = ["std", "backtrace"] }
+miden-lib = { workspace = true, features = ["std", "testing"] }
 miden-tx = { workspace = true, features = ["std", "testing"] }
+rand = { workspace = true, features = ["small_rng"] }
+winterfell = { version = "0.11" }