Implemented test for unauthenticated input notes (#408)

* tests: refactor, add tests for transaction batch

* refactor: move mock methods to `test_utils`

* tests: implement tests for batch/block producers and in-flight notes verifications

* fix: clippy warnings

* tests: fix getting unauthenticated input notes in `MockStoreSuccess`

* tests: cache mocked private accounts in order to reuse them on next creations

* tests: don't use 0's account index

* tests: fix block builder test

* format: apply rustfmt after merging

* fix: clippy warnings

* refactor: address review comments

* refactor: accept suggestion from the review

Co-authored-by: Mirko von Leipzig <[email protected]>

* tests: address review comments

---------

Co-authored-by: Mirko von Leipzig <[email protected]>

crates/block-producer/Cargo.toml

@@ -36,8 +36,11 @@ tracing-subscriber = { workspace = true }
 [dev-dependencies]
 figment = { version = "0.10", features = ["toml", "env", "test"] }
 miden-air = { workspace = true }
+miden-lib = { workspace = true, features = ["testing"] }
 miden-node-test-macro = { path = "../test-macro" }
 miden-objects = { workspace = true, features = ["testing"] }
+miden-tx = { workspace = true, features = ["testing"] }
 once_cell = { version = "1.18" }
+rand_chacha = { version = "0.3", default-features = false }
 tokio = { version = "1.38", features = ["test-util"] }
 winterfell = { version = "0.8" }

crates/block-producer/src/batch_builder/batch.rs

@@ -56,7 +56,7 @@ impl TransactionBatch {
     #[instrument(target = "miden-block-producer", name = "new_batch", skip_all, err)]
     pub fn new(
         txs: Vec<ProvenTransaction>,
-        found_unauthenticated_notes: Option<BTreeMap<NoteId, MerklePath>>,
+        found_unauthenticated_notes: BTreeMap<NoteId, MerklePath>,
     ) -> Result<Self, BuildBatchError> {
         let id = Self::compute_id(&txs);
 
@@ -81,8 +81,7 @@ impl TransactionBatch {
 
         // Populate batch produced nullifiers and match output notes with corresponding
         // unauthenticated input notes in the same batch, which are removed from the unauthenticated
-        // input notes set. We also don't add nullifiers for such output notes to the produced
-        // nullifiers set.
+        // input notes set.
         //
         // One thing to note:
         // This still allows transaction `A` to consume an unauthenticated note `x` and output note `y`
@@ -97,13 +96,12 @@ impl TransactionBatch {
                         continue;
                     }
 
-                    match found_unauthenticated_notes {
-                        Some(ref found_notes) => match found_notes.get(&input_note_header.id()) {
-                            Some(_path) => input_note.nullifier().into(),
-                            None => input_note.clone(),
-                        },
-                        None => input_note.clone(),
-                    }
+                    // If an unauthenticated note was found in the store, transform it to an authenticated one
+                    // (i.e. erase additional note details except the nullifier)
+                    found_unauthenticated_notes
+                        .get(&input_note_header.id())
+                        .map(|_path| InputNoteCommitment::from(input_note.nullifier()))
+                        .unwrap_or_else(|| input_note.clone())
                 },
                 None => input_note.clone(),
             };
@@ -193,6 +191,7 @@ impl TransactionBatch {
     }
 }
 
+#[derive(Debug)]
 struct OutputNoteTracker {
     output_notes: Vec<Option<OutputNote>>,
     output_note_index: BTreeMap<NoteId, usize>,
@@ -244,3 +243,150 @@ impl OutputNoteTracker {
         self.output_notes.into_iter().flatten().collect()
     }
 }
+
+// TESTS
+// ================================================================================================
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::{
+        mock_proven_tx,
+        note::{mock_note, mock_output_note, mock_unauthenticated_note_commitment},
+    };
+
+    #[test]
+    fn test_output_note_tracker_duplicate_output_notes() {
+        let mut txs = mock_proven_txs();
+
+        let result = OutputNoteTracker::new(&txs);
+        assert!(
+            result.is_ok(),
+            "Creation of output note tracker was not expected to fail: {result:?}"
+        );
+
+        let duplicate_output_note = txs[1].output_notes().get_note(1).clone();
+
+        txs.push(mock_proven_tx(
+            3,
+            vec![],
+            vec![duplicate_output_note.clone(), mock_output_note(8), mock_output_note(4)],
+        ));
+
+        match OutputNoteTracker::new(&txs) {
+            Err(BuildBatchError::DuplicateOutputNote(note_id, _)) => {
+                assert_eq!(note_id, duplicate_output_note.id())
+            },
+            res => panic!("Unexpected result: {res:?}"),
+        }
+    }
+
+    #[test]
+    fn test_output_note_tracker_remove_in_place_consumed_note() {
+        let txs = mock_proven_txs();
+        let mut tracker = OutputNoteTracker::new(&txs).unwrap();
+
+        let note_to_remove = mock_note(4);
+
+        assert!(tracker.remove_note(note_to_remove.header(), &txs).unwrap());
+        assert!(!tracker.remove_note(note_to_remove.header(), &txs).unwrap());
+
+        // Check that output notes are in the expected order and consumed note was removed
+        assert_eq!(
+            tracker.into_notes(),
+            vec![
+                mock_output_note(2),
+                mock_output_note(3),
+                mock_output_note(6),
+                mock_output_note(7),
+                mock_output_note(8),
+            ]
+        );
+    }
+
+    #[test]
+    fn test_duplicate_unauthenticated_notes() {
+        let mut txs = mock_proven_txs();
+        let duplicate_note = mock_note(5);
+        txs.push(mock_proven_tx(4, vec![duplicate_note.clone()], vec![mock_output_note(9)]));
+        match TransactionBatch::new(txs, Default::default()) {
+            Err(BuildBatchError::DuplicateUnauthenticatedNote(note_id, _)) => {
+                assert_eq!(note_id, duplicate_note.id())
+            },
+            res => panic!("Unexpected result: {res:?}"),
+        }
+    }
+
+    #[test]
+    fn test_consume_notes_in_place() {
+        let mut txs = mock_proven_txs();
+        let note_to_consume = mock_note(3);
+        txs.push(mock_proven_tx(
+            3,
+            vec![mock_note(11), note_to_consume, mock_note(13)],
+            vec![mock_output_note(9), mock_output_note(10)],
+        ));
+
+        let batch = TransactionBatch::new(txs, Default::default()).unwrap();
+
+        // One of the unauthenticated notes must be removed from the batch due to the consumption
+        // of the corresponding output note
+        let expected_input_notes = vec![
+            mock_unauthenticated_note_commitment(1),
+            mock_unauthenticated_note_commitment(5),
+            mock_unauthenticated_note_commitment(11),
+            mock_unauthenticated_note_commitment(13),
+        ];
+        assert_eq!(batch.input_notes, expected_input_notes);
+
+        // One of the output notes must be removed from the batch due to the consumption
+        // by the corresponding unauthenticated note
+        let expected_output_notes = vec![
+            mock_output_note(2),
+            mock_output_note(4),
+            mock_output_note(6),
+            mock_output_note(7),
+            mock_output_note(8),
+            mock_output_note(9),
+            mock_output_note(10),
+        ];
+        assert_eq!(batch.output_notes.len(), expected_output_notes.len());
+        assert_eq!(batch.output_notes, expected_output_notes);
+
+        // Ensure all nullifiers match the corresponding input notes' nullifiers
+        let expected_nullifiers: Vec<_> =
+            batch.input_notes().iter().map(InputNoteCommitment::nullifier).collect();
+        let actual_nullifiers: Vec<_> = batch.produced_nullifiers().collect();
+        assert_eq!(actual_nullifiers, expected_nullifiers);
+    }
+
+    #[test]
+    fn test_convert_unauthenticated_note_to_authenticated() {
+        let txs = mock_proven_txs();
+        let found_unauthenticated_notes =
+            BTreeMap::from_iter([(mock_note(5).id(), Default::default())]);
+        let batch = TransactionBatch::new(txs, found_unauthenticated_notes).unwrap();
+
+        let expected_input_notes =
+            vec![mock_unauthenticated_note_commitment(1), mock_note(5).nullifier().into()];
+        assert_eq!(batch.input_notes, expected_input_notes);
+    }
+
+    // UTILITIES
+    // =============================================================================================
+
+    fn mock_proven_txs() -> Vec<ProvenTransaction> {
+        vec![
+            mock_proven_tx(
+                1,
+                vec![mock_note(1)],
+                vec![mock_output_note(2), mock_output_note(3), mock_output_note(4)],
+            ),
+            mock_proven_tx(
+                2,
+                vec![mock_note(5)],
+                vec![mock_output_note(6), mock_output_note(7), mock_output_note(8)],
+            ),
+        ]
+    }
+}

crates/block-producer/src/batch_builder/mod.rs

@@ -170,7 +170,7 @@ where
         //       and only then checking against the other ready batches
         let dangling_notes = self.find_dangling_notes(&txs).await;
         let found_unauthenticated_notes = match dangling_notes.is_empty() {
-            true => None,
+            true => Default::default(),
             false => {
                 let stored_notes =
                     match self.store.get_note_authentication_info(dangling_notes.iter()).await {
@@ -186,7 +186,7 @@ where
                     return Err(BuildBatchError::UnauthenticatedNotesNotFound(missing_notes, txs));
                 }
 
-                Some(stored_notes)
+                stored_notes
             },
         };