diff --git a/Cargo.lock b/Cargo.lock index 7332ba071..557fb2851 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1595,6 +1595,7 @@ dependencies = [ "figment", "itertools 0.13.0", "miden-air", + "miden-lib", "miden-node-proto", "miden-node-store", "miden-node-test-macro", @@ -1604,6 +1605,7 @@ dependencies = [ "miden-stdlib", "miden-tx", "once_cell", + "rand_chacha", "serde", "thiserror", "tokio", @@ -1767,6 +1769,7 @@ dependencies = [ "miden-prover", "miden-verifier", "rand", + "rand_chacha", "winter-maybe-async", ] diff --git a/crates/block-producer/Cargo.toml b/crates/block-producer/Cargo.toml index cc76e910f..85d7149e3 100644 --- a/crates/block-producer/Cargo.toml +++ b/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" } diff --git a/crates/block-producer/src/batch_builder/batch.rs b/crates/block-producer/src/batch_builder/batch.rs index 010312a0f..04dbe72dc 100644 --- a/crates/block-producer/src/batch_builder/batch.rs +++ b/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, - found_unauthenticated_notes: Option>, + found_unauthenticated_notes: BTreeMap, ) -> Result { 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>, output_note_index: BTreeMap, @@ -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 { + 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)], + ), + ] + } +} diff --git a/crates/block-producer/src/batch_builder/mod.rs b/crates/block-producer/src/batch_builder/mod.rs index ab54615c4..d66c7a33d 100644 --- a/crates/block-producer/src/batch_builder/mod.rs +++ b/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 }, }; diff --git a/crates/block-producer/src/batch_builder/tests/mod.rs b/crates/block-producer/src/batch_builder/tests/mod.rs index a20686826..173c23016 100644 --- a/crates/block-producer/src/batch_builder/tests/mod.rs +++ b/crates/block-producer/src/batch_builder/tests/mod.rs @@ -4,10 +4,12 @@ use tokio::sync::RwLock; use super::*; use crate::{ + block_builder::DefaultBlockBuilder, errors::BuildBlockError, - test_utils::{MockProvenTxBuilder, MockStoreSuccessBuilder}, + test_utils::{ + note::mock_note, MockPrivateAccount, MockProvenTxBuilder, MockStoreSuccessBuilder, + }, }; - // STRUCTS // ================================================================================================ @@ -146,6 +148,159 @@ async fn test_batches_added_back_to_queue_on_block_build_failure() { assert_eq!(internal_ready_batches.read().await.len(), 3); } +#[tokio::test] +async fn test_batch_builder_find_dangling_notes() { + let store = Arc::new(MockStoreSuccessBuilder::from_accounts(iter::empty()).build()); + let block_builder = Arc::new(BlockBuilderSuccess::default()); + + let batch_builder = Arc::new(DefaultBatchBuilder::new( + store, + block_builder, + DefaultBatchBuilderOptions { + block_frequency: Duration::from_millis(20), + max_batches_per_block: 2, + }, + )); + + let note_1 = mock_note(1); + let note_2 = mock_note(2); + let tx1 = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![OutputNote::Full(note_1.clone())]) + .build(); + let tx2 = MockProvenTxBuilder::with_account_index(1) + .unauthenticated_notes(vec![note_1.clone()]) + .output_notes(vec![OutputNote::Full(note_2.clone())]) + .build(); + + let txs = vec![tx1, tx2]; + + let dangling_notes = batch_builder.find_dangling_notes(&txs).await; + assert_eq!(dangling_notes, vec![], "Note must be presented in the same batch"); + + batch_builder.build_batch(txs.clone()).await.unwrap(); + + let dangling_notes = batch_builder.find_dangling_notes(&txs).await; + assert_eq!(dangling_notes, vec![], "Note must be presented in the same batch"); + + let note_3 = mock_note(3); + + let tx1 = MockProvenTxBuilder::with_account_index(1) + .unauthenticated_notes(vec![note_2.clone()]) + .build(); + let tx2 = MockProvenTxBuilder::with_account_index(1) + .unauthenticated_notes(vec![note_3.clone()]) + .build(); + + let txs = vec![tx1, tx2]; + + let dangling_notes = batch_builder.find_dangling_notes(&txs).await; + assert_eq!( + dangling_notes, + vec![note_3.id()], + "Only one dangling node must be found before block is built" + ); + + batch_builder.try_build_block().await; + + let dangling_notes = batch_builder.find_dangling_notes(&txs).await; + assert_eq!( + dangling_notes, + vec![note_2.id(), note_3.id()], + "Two dangling notes must be found after block is built" + ); +} + +#[tokio::test] +async fn test_block_builder_no_missing_notes() { + let account_1: MockPrivateAccount<3> = MockPrivateAccount::from(1); + let account_2: MockPrivateAccount<3> = MockPrivateAccount::from(2); + let store = Arc::new( + MockStoreSuccessBuilder::from_accounts( + [account_1, account_2].iter().map(|account| (account.id, account.states[0])), + ) + .build(), + ); + let block_builder = Arc::new(DefaultBlockBuilder::new(Arc::clone(&store), Arc::clone(&store))); + let batch_builder = Arc::new(DefaultBatchBuilder::new( + store, + Arc::clone(&block_builder), + DefaultBatchBuilderOptions { + block_frequency: Duration::from_millis(20), + max_batches_per_block: 2, + }, + )); + + let note_1 = mock_note(1); + let note_2 = mock_note(2); + + let tx1 = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![OutputNote::Full(note_1.clone())]) + .build(); + + let tx2 = MockProvenTxBuilder::with_account_index(2) + .unauthenticated_notes(vec![note_1.clone()]) + .output_notes(vec![OutputNote::Full(note_2.clone())]) + .build(); + + let txs = vec![tx1, tx2]; + + batch_builder.build_batch(txs.clone()).await.unwrap(); + + let build_block_result = batch_builder + .block_builder + .build_block(&batch_builder.ready_batches.read().await) + .await; + assert_eq!(build_block_result, Ok(())); +} + +#[tokio::test] +async fn test_block_builder_fails_if_notes_are_missing() { + let accounts: Vec<_> = (1..=4).map(MockPrivateAccount::<3>::from).collect(); + let notes: Vec<_> = (1..=6).map(mock_note).collect(); + + let store = Arc::new( + MockStoreSuccessBuilder::from_accounts( + accounts.iter().map(|account| (account.id, account.states[0])), + ) + .initial_notes([vec![OutputNote::Full(notes[0].clone())]].iter()) + .build(), + ); + let block_builder = Arc::new(DefaultBlockBuilder::new(Arc::clone(&store), Arc::clone(&store))); + let batch_builder = Arc::new(DefaultBatchBuilder::new( + store, + Arc::clone(&block_builder), + DefaultBatchBuilderOptions { + block_frequency: Duration::from_millis(20), + max_batches_per_block: 2, + }, + )); + + let tx1 = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![OutputNote::Full(notes[1].clone())]) + .build(); + + let tx2 = MockProvenTxBuilder::with_account_index(2) + .unauthenticated_notes(vec![notes[0].clone()]) + .output_notes(vec![OutputNote::Full(notes[2].clone()), OutputNote::Full(notes[3].clone())]) + .build(); + + let tx3 = MockProvenTxBuilder::with_account_index(3) + .unauthenticated_notes(notes.iter().skip(1).cloned().collect()) + .build(); + + let txs = vec![tx1, tx2, tx3]; + + let batch = TransactionBatch::new(txs.clone(), Default::default()).unwrap(); + let build_block_result = batch_builder.block_builder.build_block(&[batch]).await; + assert_eq!( + build_block_result, + Err(BuildBlockError::UnauthenticatedNotesNotFound(vec![ + notes[4].id(), + notes[5].id() + ])) + ); +} + // HELPERS // ================================================================================================ @@ -155,5 +310,5 @@ fn dummy_tx_batch(starting_account_index: u32, num_txs_in_batch: usize) -> Trans MockProvenTxBuilder::with_account_index(starting_account_index + index as u32).build() }) .collect(); - TransactionBatch::new(txs, None).unwrap() + TransactionBatch::new(txs, Default::default()).unwrap() } diff --git a/crates/block-producer/src/block_builder/mod.rs b/crates/block-producer/src/block_builder/mod.rs index 0c63ffae4..b6a64bd3e 100644 --- a/crates/block-producer/src/block_builder/mod.rs +++ b/crates/block-producer/src/block_builder/mod.rs @@ -109,13 +109,12 @@ where ) .await?; - if block_inputs.found_unauthenticated_notes.len() < dangling_notes.len() { - return Err(BuildBlockError::UnauthenticatedNotesNotFound( - dangling_notes - .difference(&block_inputs.found_unauthenticated_notes) - .copied() - .collect(), - )); + let missing_notes: Vec<_> = dangling_notes + .difference(&block_inputs.found_unauthenticated_notes) + .copied() + .collect(); + if !missing_notes.is_empty() { + return Err(BuildBlockError::UnauthenticatedNotesNotFound(missing_notes)); } let block_header_witness = BlockWitness::new(block_inputs, batches)?; diff --git a/crates/block-producer/src/block_builder/prover/tests.rs b/crates/block-producer/src/block_builder/prover/tests.rs index a0cf4ea23..1c7580761 100644 --- a/crates/block-producer/src/block_builder/prover/tests.rs +++ b/crates/block-producer/src/block_builder/prover/tests.rs @@ -69,7 +69,7 @@ fn test_block_witness_validation_inconsistent_account_ids() { ) .build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; let batch_2 = { @@ -80,7 +80,7 @@ fn test_block_witness_validation_inconsistent_account_ids() { ) .build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; vec![batch_1, batch_2] @@ -141,7 +141,7 @@ fn test_block_witness_validation_inconsistent_account_hashes() { Digest::default(), ) .build()], - None, + Default::default(), ) .unwrap(); let batch_2 = TransactionBatch::new( @@ -151,7 +151,7 @@ fn test_block_witness_validation_inconsistent_account_hashes() { Digest::default(), ) .build()], - None, + Default::default(), ) .unwrap(); @@ -235,8 +235,8 @@ async fn test_compute_account_root_success() { }) .collect(); - let batch_1 = TransactionBatch::new(txs[..2].to_vec(), None).unwrap(); - let batch_2 = TransactionBatch::new(txs[2..].to_vec(), None).unwrap(); + let batch_1 = TransactionBatch::new(txs[..2].to_vec(), Default::default()).unwrap(); + let batch_2 = TransactionBatch::new(txs[2..].to_vec(), Default::default()).unwrap(); vec![batch_1, batch_2] }; @@ -379,7 +379,7 @@ async fn test_compute_note_root_empty_notes_success() { .unwrap(); let batches: Vec = { - let batch = TransactionBatch::new(vec![], None).unwrap(); + let batch = TransactionBatch::new(vec![], Default::default()).unwrap(); vec![batch] }; @@ -447,13 +447,13 @@ async fn test_compute_note_root_success() { .map(|(note, &account_id)| { let note = OutputNote::Header(*note); MockProvenTxBuilder::with_account(account_id, Digest::default(), Digest::default()) - .notes_created(vec![note]) + .output_notes(vec![note]) .build() }) .collect(); - let batch_1 = TransactionBatch::new(txs[..2].to_vec(), None).unwrap(); - let batch_2 = TransactionBatch::new(txs[2..].to_vec(), None).unwrap(); + let batch_1 = TransactionBatch::new(txs[..2].to_vec(), Default::default()).unwrap(); + let batch_2 = TransactionBatch::new(txs[2..].to_vec(), Default::default()).unwrap(); vec![batch_1, batch_2] }; @@ -501,13 +501,13 @@ fn test_block_witness_validation_inconsistent_nullifiers() { let batch_1 = { let tx = MockProvenTxBuilder::with_account_index(0).nullifiers_range(0..1).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; let batch_2 = { let tx = MockProvenTxBuilder::with_account_index(1).nullifiers_range(1..2).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; vec![batch_1, batch_2] @@ -577,13 +577,13 @@ async fn test_compute_nullifier_root_empty_success() { let batch_1 = { let tx = MockProvenTxBuilder::with_account_index(0).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; let batch_2 = { let tx = MockProvenTxBuilder::with_account_index(1).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; vec![batch_1, batch_2] @@ -630,13 +630,13 @@ async fn test_compute_nullifier_root_success() { let batch_1 = { let tx = MockProvenTxBuilder::with_account_index(0).nullifiers_range(0..1).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; let batch_2 = { let tx = MockProvenTxBuilder::with_account_index(1).nullifiers_range(1..2).build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; vec![batch_1, batch_2] diff --git a/crates/block-producer/src/block_builder/tests.rs b/crates/block-producer/src/block_builder/tests.rs index b146c66d6..c7c82629f 100644 --- a/crates/block-producer/src/block_builder/tests.rs +++ b/crates/block-producer/src/block_builder/tests.rs @@ -37,7 +37,7 @@ async fn test_apply_block_called_nonempty_batches() { ) .build(); - TransactionBatch::new(vec![tx], None).unwrap() + TransactionBatch::new(vec![tx], Default::default()).unwrap() }; vec![batch_1] diff --git a/crates/block-producer/src/state_view/tests/verify_tx.rs b/crates/block-producer/src/state_view/tests/verify_tx.rs index ee6bf3155..faec60205 100644 --- a/crates/block-producer/src/state_view/tests/verify_tx.rs +++ b/crates/block-producer/src/state_view/tests/verify_tx.rs @@ -12,10 +12,11 @@ use std::iter; +use miden_objects::notes::Note; use tokio::task::JoinSet; use super::*; -use crate::test_utils::MockStoreSuccessBuilder; +use crate::test_utils::{block::MockBlockBuilder, note::mock_note, MockStoreSuccessBuilder}; /// Tests the happy path where 3 transactions who modify different accounts and consume different /// notes all verify successfully @@ -198,7 +199,7 @@ async fn test_verify_tx_vt5() { // Notice: `consumed_note_in_both_txs` is NOT in the store let store = Arc::new( MockStoreSuccessBuilder::from_accounts( - vec![account_1, account_2] + [account_1, account_2] .into_iter() .map(|account| (account.id, account.states[0])), ) @@ -228,3 +229,80 @@ async fn test_verify_tx_vt5() { Err(VerifyTxError::InputNotesAlreadyConsumed(vec![nullifier_in_both_txs])) ); } + +/// Tests that `verify_tx()` succeeds when the unauthenticated input note found in the in-flight notes +#[tokio::test] +#[miden_node_test_macro::enable_logging] +async fn test_verify_tx_dangling_note_found_in_inflight_notes() { + let account_1: MockPrivateAccount<3> = MockPrivateAccount::from(1); + let account_2: MockPrivateAccount<3> = MockPrivateAccount::from(2); + let store = Arc::new( + MockStoreSuccessBuilder::from_accounts( + [account_1, account_2] + .into_iter() + .map(|account| (account.id, account.states[0])), + ) + .build(), + ); + let state_view = DefaultStateView::new(Arc::clone(&store), false); + + let dangling_notes = vec![mock_note(1)]; + let output_notes = dangling_notes.iter().cloned().map(OutputNote::Full).collect(); + + let tx1 = MockProvenTxBuilder::with_account_index(1).output_notes(output_notes).build(); + + let verify_tx1_result = state_view.verify_tx(&tx1).await; + assert_eq!(verify_tx1_result, Ok(())); + + let tx2 = MockProvenTxBuilder::with_account_index(2) + .unauthenticated_notes(dangling_notes.clone()) + .build(); + + let verify_tx2_result = state_view.verify_tx(&tx2).await; + assert_eq!( + verify_tx2_result, + Ok(()), + "Dangling unauthenticated notes must be found in the in-flight notes after previous tx verification" + ); +} + +/// Tests that `verify_tx()` fails when the unauthenticated input note not found not in the in-flight +/// notes nor in the store +#[tokio::test] +#[miden_node_test_macro::enable_logging] +async fn test_verify_tx_stored_unauthenticated_notes() { + let account_1: MockPrivateAccount<3> = MockPrivateAccount::from(1); + let store = Arc::new( + MockStoreSuccessBuilder::from_accounts( + [account_1].into_iter().map(|account| (account.id, account.states[0])), + ) + .build(), + ); + let dangling_notes = vec![mock_note(1)]; + let tx1 = MockProvenTxBuilder::with_account_index(1) + .unauthenticated_notes(dangling_notes.clone()) + .build(); + + let state_view = DefaultStateView::new(Arc::clone(&store), false); + + let verify_tx1_result = state_view.verify_tx(&tx1).await; + assert_eq!( + verify_tx1_result, + Err(VerifyTxError::UnauthenticatedNotesNotFound( + dangling_notes.iter().map(Note::id).collect() + )), + "Dangling unauthenticated notes must not be found in the store by this moment" + ); + + let output_notes = dangling_notes.into_iter().map(OutputNote::Full).collect(); + let block = MockBlockBuilder::new(&store).await.created_notes(vec![output_notes]).build(); + + store.apply_block(&block).await.unwrap(); + + let verify_tx1_result = state_view.verify_tx(&tx1).await; + assert_eq!( + verify_tx1_result, + Ok(()), + "Dangling unauthenticated notes must be found in the store after block applying" + ); +} diff --git a/crates/block-producer/src/test_utils/account.rs b/crates/block-producer/src/test_utils/account.rs index 13e8f985a..0facc1bda 100644 --- a/crates/block-producer/src/test_utils/account.rs +++ b/crates/block-producer/src/test_utils/account.rs @@ -1,10 +1,16 @@ +use std::{collections::HashMap, ops::Not}; + use miden_objects::{ accounts::{get_account_seed, AccountStorageType, AccountType}, Hasher, }; +use once_cell::sync::Lazy; use super::*; +pub static MOCK_ACCOUNTS: Lazy>> = + Lazy::new(Default::default); + /// A mock representation fo private accounts. An account starts in state `states[0]`, is modified /// to state `states[1]`, and so on. #[derive(Clone, Copy, Debug)] @@ -16,7 +22,19 @@ pub struct MockPrivateAccount { } impl MockPrivateAccount { - fn new(init_seed: [u8; 32], new_account: bool) -> Self { + fn new(id: AccountId, initial_state: Digest) -> Self { + let mut states = [Digest::default(); NUM_STATES]; + + states[0] = initial_state; + + for idx in 1..NUM_STATES { + states[idx] = Hasher::hash(&states[idx - 1].as_bytes()); + } + + Self { id, states } + } + + fn generate(init_seed: [u8; 32], new_account: bool) -> Self { let account_seed = get_account_seed( init_seed, AccountType::RegularAccountUpdatableCode, @@ -26,20 +44,10 @@ impl MockPrivateAccount { ) .unwrap(); - let mut states = [Digest::default(); NUM_STATES]; - - if !new_account { - states[0] = Hasher::hash(&init_seed); - } - - for idx in 1..NUM_STATES { - states[idx] = Hasher::hash(&states[idx - 1].as_bytes()); - } - - Self { - id: AccountId::new(account_seed, Digest::default(), Digest::default()).unwrap(), - states, - } + Self::new( + AccountId::new(account_seed, Digest::default(), Digest::default()).unwrap(), + new_account.not().then(|| Hasher::hash(&init_seed)).unwrap_or_default(), + ) } } @@ -47,13 +55,26 @@ impl From for MockPrivateAccount { /// Each index gives rise to a different account ID /// Passing index 0 signifies that it's a new account fn from(index: u32) -> Self { + let mut lock = MOCK_ACCOUNTS.lock().expect("Poisoned mutex"); + if let Some(&(account_id, init_state)) = lock.get(&index) { + return Self::new(account_id, init_state); + } + let init_seed: Vec<_> = index.to_be_bytes().into_iter().chain([0u8; 28]).collect(); // using index 0 signifies that it's a new account - if index == 0 { - Self::new(init_seed.try_into().unwrap(), true) + let account = if index == 0 { + Self::generate(init_seed.try_into().unwrap(), true) } else { - Self::new(init_seed.try_into().unwrap(), false) - } + Self::generate(init_seed.try_into().unwrap(), false) + }; + + lock.insert(index, (account.id, account.states[0])); + + account } } + +pub fn mock_account_id(num: u8) -> AccountId { + MockPrivateAccount::<3>::from(num as u32).id +} diff --git a/crates/block-producer/src/test_utils/batch.rs b/crates/block-producer/src/test_utils/batch.rs index f340824d9..889fc8829 100644 --- a/crates/block-producer/src/test_utils/batch.rs +++ b/crates/block-producer/src/test_utils/batch.rs @@ -24,7 +24,7 @@ impl TransactionBatchConstructor for TransactionBatch { }) .collect(); - Self::new(txs, None).unwrap() + Self::new(txs, Default::default()).unwrap() } fn from_txs(starting_account_index: u32, num_txs_in_batch: u64) -> Self { @@ -36,6 +36,6 @@ impl TransactionBatchConstructor for TransactionBatch { }) .collect(); - Self::new(txs, None).unwrap() + Self::new(txs, Default::default()).unwrap() } } diff --git a/crates/block-producer/src/test_utils/block.rs b/crates/block-producer/src/test_utils/block.rs index 9bba6e5bf..44c961014 100644 --- a/crates/block-producer/src/test_utils/block.rs +++ b/crates/block-producer/src/test_utils/block.rs @@ -45,6 +45,8 @@ pub async fn build_expected_block_header( store_chain_mmr.peaks(store_chain_mmr.forest()).unwrap().hash_peaks() }; + let note_created_smt = note_created_smt_from_note_batches(block_output_notes(batches.iter())); + // Build header BlockHeader::new( 0, @@ -54,7 +56,7 @@ pub async fn build_expected_block_header( new_account_root, // FIXME: FILL IN CORRECT NULLIFIER ROOT Digest::default(), - note_created_smt_from_batches(batches).root(), + note_created_smt.root(), Digest::default(), Digest::default(), 1, @@ -93,7 +95,7 @@ pub struct MockBlockBuilder { last_block_header: BlockHeader, updated_accounts: Option>, - created_note: Option>, + created_notes: Option>, produced_nullifiers: Option>, } @@ -105,7 +107,7 @@ impl MockBlockBuilder { last_block_header: *store.last_block_header.read().await, updated_accounts: None, - created_note: None, + created_notes: None, produced_nullifiers: None, } } @@ -121,6 +123,12 @@ impl MockBlockBuilder { self } + pub fn created_notes(mut self, created_notes: Vec) -> Self { + self.created_notes = Some(created_notes); + + self + } + pub fn produced_nullifiers(mut self, produced_nullifiers: Vec) -> Self { self.produced_nullifiers = Some(produced_nullifiers); @@ -128,7 +136,7 @@ impl MockBlockBuilder { } pub fn build(self) -> Block { - let created_notes = self.created_note.unwrap_or_default(); + let created_notes = self.created_notes.unwrap_or_default(); let header = BlockHeader::new( 0, @@ -153,22 +161,27 @@ impl MockBlockBuilder { } } +pub(crate) fn flatten_output_notes<'a>( + batches: impl Iterator, +) -> impl Iterator { + batches.enumerate().flat_map(|(batch_idx, batch)| { + batch.iter().enumerate().map(move |(note_idx_in_batch, note)| { + (BlockNoteIndex::new(batch_idx, note_idx_in_batch), note) + }) + }) +} + pub(crate) fn note_created_smt_from_note_batches<'a>( - batches: impl Iterator + Clone + 'a)>, + batches: impl Iterator, ) -> BlockNoteTree { - let note_leaf_iterator = batches.enumerate().flat_map(|(batch_idx, batch)| { - batch.clone().into_iter().enumerate().map(move |(note_idx_in_batch, note)| { - ( - BlockNoteIndex::new(batch_idx, note_idx_in_batch), - note.id().into(), - *note.metadata(), - ) - }) - }); + let note_leaf_iterator = flatten_output_notes(batches) + .map(|(index, note)| (index, note.id().into(), *note.metadata())); BlockNoteTree::with_entries(note_leaf_iterator).unwrap() } -pub(crate) fn note_created_smt_from_batches(batches: &[TransactionBatch]) -> BlockNoteTree { - note_created_smt_from_note_batches(batches.iter().map(TransactionBatch::output_notes)) +pub(crate) fn block_output_notes<'a>( + batches: impl Iterator + Clone, +) -> impl Iterator + Clone { + batches.map(TransactionBatch::output_notes) } diff --git a/crates/block-producer/src/test_utils/mod.rs b/crates/block-producer/src/test_utils/mod.rs index bd0a63afd..07a7a4b52 100644 --- a/crates/block-producer/src/test_utils/mod.rs +++ b/crates/block-producer/src/test_utils/mod.rs @@ -5,7 +5,7 @@ use tokio::sync::RwLock; mod proven_tx; -pub use proven_tx::MockProvenTxBuilder; +pub use proven_tx::{mock_proven_tx, MockProvenTxBuilder}; mod store; @@ -13,8 +13,10 @@ pub use store::{MockStoreFailure, MockStoreSuccess, MockStoreSuccessBuilder}; mod account; -pub use account::MockPrivateAccount; +pub use account::{mock_account_id, MockPrivateAccount}; pub mod block; pub mod batch; + +pub mod note; diff --git a/crates/block-producer/src/test_utils/note.rs b/crates/block-producer/src/test_utils/note.rs new file mode 100644 index 000000000..0a3b0a7d2 --- /dev/null +++ b/crates/block-producer/src/test_utils/note.rs @@ -0,0 +1,24 @@ +use miden_lib::transaction::TransactionKernel; +use miden_objects::{ + notes::Note, + testing::notes::NoteBuilder, + transaction::{InputNote, InputNoteCommitment, OutputNote}, +}; +use rand_chacha::{rand_core::SeedableRng, ChaCha20Rng}; + +use crate::test_utils::account::mock_account_id; + +pub fn mock_note(num: u8) -> Note { + let sender = mock_account_id(num); + NoteBuilder::new(sender, ChaCha20Rng::from_seed([num; 32])) + .build(&TransactionKernel::assembler().with_debug_mode(true)) + .unwrap() +} + +pub fn mock_unauthenticated_note_commitment(num: u8) -> InputNoteCommitment { + InputNote::unauthenticated(mock_note(num)).into() +} + +pub fn mock_output_note(num: u8) -> OutputNote { + OutputNote::Full(mock_note(num)) +} diff --git a/crates/block-producer/src/test_utils/proven_tx.rs b/crates/block-producer/src/test_utils/proven_tx.rs index 92ffa5369..3b9cce2c1 100644 --- a/crates/block-producer/src/test_utils/proven_tx.rs +++ b/crates/block-producer/src/test_utils/proven_tx.rs @@ -3,8 +3,8 @@ use std::ops::Range; use miden_air::HashFunction; use miden_objects::{ accounts::AccountId, - notes::{NoteHeader, NoteMetadata, NoteType, Nullifier}, - transaction::{OutputNote, ProvenTransaction, ProvenTransactionBuilder}, + notes::{Note, NoteHeader, NoteMetadata, NoteType, Nullifier}, + transaction::{InputNote, OutputNote, ProvenTransaction, ProvenTransactionBuilder}, vm::ExecutionProof, Digest, Felt, Hasher, ONE, }; @@ -16,7 +16,8 @@ pub struct MockProvenTxBuilder { account_id: AccountId, initial_account_hash: Digest, final_account_hash: Digest, - notes_created: Option>, + output_notes: Option>, + input_notes: Option>, nullifiers: Option>, } @@ -36,19 +37,26 @@ impl MockProvenTxBuilder { account_id, initial_account_hash, final_account_hash, - notes_created: None, + output_notes: None, + input_notes: None, nullifiers: None, } } + pub fn unauthenticated_notes(mut self, notes: Vec) -> Self { + self.input_notes = Some(notes.into_iter().map(InputNote::unauthenticated).collect()); + + self + } + pub fn nullifiers(mut self, nullifiers: Vec) -> Self { self.nullifiers = Some(nullifiers); self } - pub fn notes_created(mut self, notes: Vec) -> Self { - self.notes_created = Some(notes); + pub fn output_notes(mut self, notes: Vec) -> Self { + self.output_notes = Some(notes); self } @@ -76,7 +84,7 @@ impl MockProvenTxBuilder { }) .collect(); - self.notes_created(notes) + self.output_notes(notes) } pub fn build(self) -> ProvenTransaction { @@ -87,9 +95,21 @@ impl MockProvenTxBuilder { Digest::default(), ExecutionProof::new(StarkProof::new_dummy(), HashFunction::Blake3_192), ) - .add_input_notes(self.nullifiers.unwrap_or_default().iter().copied()) - .add_output_notes(self.notes_created.unwrap_or_default()) + .add_input_notes(self.input_notes.unwrap_or_default()) + .add_input_notes(self.nullifiers.unwrap_or_default()) + .add_output_notes(self.output_notes.unwrap_or_default()) .build() .unwrap() } } + +pub fn mock_proven_tx( + account_index: u8, + unauthenticated_notes: Vec, + output_notes: Vec, +) -> ProvenTransaction { + MockProvenTxBuilder::with_account_index(account_index.into()) + .unauthenticated_notes(unauthenticated_notes) + .output_notes(output_notes) + .build() +} diff --git a/crates/block-producer/src/test_utils/store.rs b/crates/block-producer/src/test_utils/store.rs index 8d684fb7b..d3d6a3ed7 100644 --- a/crates/block-producer/src/test_utils/store.rs +++ b/crates/block-producer/src/test_utils/store.rs @@ -1,14 +1,14 @@ use std::{ collections::{BTreeMap, BTreeSet}, num::NonZeroU32, + ops::Not, }; use async_trait::async_trait; use miden_objects::{ - block::{Block, BlockNoteTree}, + block::{Block, BlockNoteTree, NoteBatch}, crypto::merkle::{MerklePath, Mmr, SimpleSmt, Smt, ValuePath}, notes::{NoteId, Nullifier}, - transaction::OutputNote, BlockHeader, ACCOUNT_TREE_DEPTH, EMPTY_WORD, ZERO, }; @@ -20,7 +20,9 @@ use crate::{ store::{ ApplyBlock, ApplyBlockError, BlockInputsError, Store, TransactionInputs, TxInputsError, }, - test_utils::block::{note_created_smt_from_batches, note_created_smt_from_note_batches}, + test_utils::block::{ + block_output_notes, flatten_output_notes, note_created_smt_from_note_batches, + }, ProvenTransaction, }; @@ -28,29 +30,31 @@ use crate::{ #[derive(Debug)] pub struct MockStoreSuccessBuilder { accounts: Option>, - notes: Option, + notes: Option>, + note_root: Option, produced_nullifiers: Option>, chain_mmr: Option, block_num: Option, } impl MockStoreSuccessBuilder { - pub fn from_batches<'a>(batches: impl Iterator) -> Self { - let batches: Vec<_> = batches.cloned().collect(); - + pub fn from_batches<'a>( + batches_iter: impl Iterator + Clone, + ) -> Self { let accounts_smt = { - let accounts = batches - .iter() + let accounts = batches_iter + .clone() .flat_map(TransactionBatch::account_initial_states) .map(|(account_id, hash)| (account_id.into(), hash.into())); SimpleSmt::::with_leaves(accounts).unwrap() }; - let created_notes = note_created_smt_from_batches(&batches); + let (note_tree, notes) = Self::populate_note_trees(block_output_notes(batches_iter)); Self { accounts: Some(accounts_smt), - notes: Some(created_notes), + notes: Some(notes), + note_root: Some(note_tree.root()), produced_nullifiers: None, chain_mmr: None, block_num: None, @@ -67,17 +71,18 @@ impl MockStoreSuccessBuilder { Self { accounts: Some(accounts_smt), notes: None, + note_root: None, produced_nullifiers: None, chain_mmr: None, block_num: None, } } - pub fn initial_notes<'a>( - mut self, - notes: impl Iterator + Clone + 'a)>, - ) -> Self { - self.notes = Some(note_created_smt_from_note_batches(notes)); + pub fn initial_notes<'a>(mut self, notes: impl Iterator + Clone) -> Self { + let (note_tree, notes) = Self::populate_note_trees(notes); + + self.notes = Some(notes); + self.note_root = Some(note_tree.root()); self } @@ -100,10 +105,22 @@ impl MockStoreSuccessBuilder { self } + fn populate_note_trees<'a>( + batches_iterator: impl Iterator + Clone, + ) -> (BlockNoteTree, BTreeMap) { + let block_note_tree = note_created_smt_from_note_batches(batches_iterator.clone()); + let note_map = flatten_output_notes(batches_iterator) + .map(|(index, note)| (note.id(), block_note_tree.get_note_path(index).unwrap())) + .collect(); + + (block_note_tree, note_map) + } + pub fn build(self) -> MockStoreSuccess { let block_num = self.block_num.unwrap_or(1); let accounts_smt = self.accounts.unwrap_or(SimpleSmt::new().unwrap()); - let notes_smt = self.notes.unwrap_or_default(); + let notes = self.notes.unwrap_or_default(); + let note_root = self.note_root.unwrap_or_default(); let chain_mmr = self.chain_mmr.unwrap_or_default(); let nullifiers_smt = self .produced_nullifiers @@ -124,7 +141,7 @@ impl MockStoreSuccessBuilder { chain_mmr.peaks(chain_mmr.forest()).unwrap().hash_peaks(), accounts_smt.root(), nullifiers_smt.root(), - notes_smt.root(), + note_root, Digest::default(), Digest::default(), 1, @@ -135,7 +152,8 @@ impl MockStoreSuccessBuilder { produced_nullifiers: Arc::new(RwLock::new(nullifiers_smt)), chain_mmr: Arc::new(RwLock::new(chain_mmr)), last_block_header: Arc::new(RwLock::new(initial_block_header)), - num_apply_block_called: Arc::new(RwLock::new(0)), + num_apply_block_called: Default::default(), + notes: Arc::new(RwLock::new(notes)), } } } @@ -155,6 +173,9 @@ pub struct MockStoreSuccess { /// The number of times `apply_block()` was called pub num_apply_block_called: Arc>, + + /// Maps note id -> note inclusion proof for all created notes + pub notes: Arc>>, } impl MockStoreSuccess { @@ -191,6 +212,15 @@ impl ApplyBlock for MockStoreSuccess { chain_mmr.add(block.hash()); } + // build note tree + let note_tree = block.build_note_tree(); + + // update notes + let mut locked_notes = self.notes.write().await; + for (note_index, note) in block.notes() { + locked_notes.insert(note.id(), note_tree.get_note_path(note_index).unwrap_or_default()); + } + // update last block header *self.last_block_header.write().await = block.header(); @@ -231,11 +261,20 @@ impl Store for MockStoreSuccess { }) .collect(); + let locked_notes = self.notes.read().await; + let missing_unauthenticated_notes = proven_tx + .get_unauthenticated_notes() + .filter_map(|header| { + let id = header.id(); + locked_notes.contains_key(&id).not().then_some(id) + }) + .collect(); + Ok(TransactionInputs { account_id: proven_tx.account_id(), account_hash, nullifiers, - missing_unauthenticated_notes: Default::default(), + missing_unauthenticated_notes, }) } @@ -268,7 +307,9 @@ impl Store for MockStoreSuccess { .map(|nullifier| (*nullifier, locked_produced_nullifiers.open(&nullifier.inner()))) .collect(); - let found_unauthenticated_notes = notes.copied().collect(); + let locked_notes = self.notes.read().await; + let found_unauthenticated_notes = + notes.filter(|&id| locked_notes.contains_key(id)).copied().collect(); Ok(BlockInputs { block_header: *self.last_block_header.read().await, @@ -281,9 +322,14 @@ impl Store for MockStoreSuccess { async fn get_note_authentication_info( &self, - _notes: impl Iterator + Send, + notes: impl Iterator + Send, ) -> Result, NotePathsError> { - todo!() + let locked_notes = self.notes.read().await; + let note_auth_info = notes + .filter_map(|note_id| locked_notes.get(note_id).map(|path| (*note_id, path.clone()))) + .collect(); + + Ok(note_auth_info) } } diff --git a/crates/block-producer/src/txqueue/tests/mod.rs b/crates/block-producer/src/txqueue/tests/mod.rs index 079290449..49ac130e6 100644 --- a/crates/block-producer/src/txqueue/tests/mod.rs +++ b/crates/block-producer/src/txqueue/tests/mod.rs @@ -40,8 +40,8 @@ impl BatchBuilderSuccess { #[async_trait] impl BatchBuilder for BatchBuilderSuccess { async fn build_batch(&self, txs: Vec) -> Result<(), BuildBatchError> { - let batch = - TransactionBatch::new(txs, None).expect("Tx batch building should have succeeded"); + let batch = TransactionBatch::new(txs, Default::default()) + .expect("Tx batch building should have succeeded"); self.ready_batches .send(batch) .expect("Sending to channel should have succeeded"); @@ -105,7 +105,8 @@ async fn test_build_batch_success() { receiver.try_recv(), "A single transaction produces a single batch" ); - let expected = TransactionBatch::new(vec![tx.clone()], None).expect("Valid transactions"); + let expected = + TransactionBatch::new(vec![tx.clone()], Default::default()).expect("Valid transactions"); assert_eq!(expected, batch, "The batch should have the one transaction added to the queue"); // a batch will include up to `batch_size` transactions @@ -124,7 +125,7 @@ async fn test_build_batch_success() { receiver.try_recv(), "{batch_size} transactions create a single batch" ); - let expected = TransactionBatch::new(txs, None).expect("Valid transactions"); + let expected = TransactionBatch::new(txs, Default::default()).expect("Valid transactions"); assert_eq!(expected, batch, "The batch should the transactions to fill a batch"); // the transaction queue eagerly produces batches @@ -139,7 +140,8 @@ async fn test_build_batch_success() { for expected_batch in txs.chunks(batch_size).map(|txs| txs.to_vec()) { tokio::time::advance(build_batch_frequency).await; let batch = receiver.try_recv().expect("Queue not empty"); - let expected = TransactionBatch::new(expected_batch, None).expect("Valid transactions"); + let expected = + TransactionBatch::new(expected_batch, Default::default()).expect("Valid transactions"); assert_eq!(expected, batch, "The batch should the transactions to fill a batch"); }