From 60cb5756172abc1cba843ee01d77039088372578 Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Mon, 26 Feb 2024 09:39:13 +0500 Subject: [PATCH 1/4] WIP: created notes root --- .../src/block_builder/prover/block_witness.rs | 3 +- block-producer/src/errors.rs | 2 + block-producer/src/test_utils/block.rs | 21 ++++++--- block-producer/src/test_utils/store.rs | 46 +++++++++++++++++-- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/block-producer/src/block_builder/prover/block_witness.rs b/block-producer/src/block_builder/prover/block_witness.rs index 3da473e67..87bcbcd0e 100644 --- a/block-producer/src/block_builder/prover/block_witness.rs +++ b/block-producer/src/block_builder/prover/block_witness.rs @@ -225,7 +225,6 @@ impl BlockWitness { // Notes stack inputs { - let num_created_notes_roots = self.batch_created_notes_roots.len(); for (batch_index, batch_created_notes_root) in self.batch_created_notes_roots.iter() { stack_inputs.extend(batch_created_notes_root.iter()); @@ -237,7 +236,7 @@ impl BlockWitness { let empty_root = EmptySubtreeRoots::entry(BLOCK_OUTPUT_NOTES_TREE_DEPTH, 0); stack_inputs.extend(*empty_root); stack_inputs.push( - Felt::try_from(num_created_notes_roots as u64) + Felt::try_from(self.batch_created_notes_roots.len() as u64) .expect("notes roots number is greater than or equal to the field modulus"), ); } diff --git a/block-producer/src/errors.rs b/block-producer/src/errors.rs index d52021437..b99e8287f 100644 --- a/block-producer/src/errors.rs +++ b/block-producer/src/errors.rs @@ -113,6 +113,8 @@ pub enum BlockInputsError { #[derive(Debug, PartialEq, Eq, Error)] pub enum ApplyBlockError { + #[error("Merkle error: {0}")] + MerkleError(#[from] MerkleError), #[error("gRPC client failed with error: {0}")] GrpcClientError(String), } diff --git a/block-producer/src/test_utils/block.rs b/block-producer/src/test_utils/block.rs index 8ee9177ad..446e56b7e 100644 --- a/block-producer/src/test_utils/block.rs +++ b/block-producer/src/test_utils/block.rs @@ -4,7 +4,7 @@ use miden_objects::{ accounts::AccountId, crypto::merkle::Mmr, notes::{NoteEnvelope, Nullifier}, - BlockHeader, Digest, ACCOUNT_TREE_DEPTH, ONE, ZERO, + BlockHeader, Digest, Word, ACCOUNT_TREE_DEPTH, NOTE_TREE_DEPTH, ONE, ZERO, }; use miden_vm::crypto::SimpleSmt; @@ -26,10 +26,10 @@ pub async fn build_expected_block_header( // Compute new account root let updated_accounts: Vec<(AccountId, Digest)> = - batches.iter().flat_map(|batch| batch.updated_accounts()).collect(); + batches.iter().flat_map(TransactionBatch::updated_accounts).collect(); let new_account_root = { let mut store_accounts = store.accounts.read().await.clone(); - for &(account_id, new_account_state) in updated_accounts.iter() { + for (account_id, new_account_state) in updated_accounts { store_accounts.insert(account_id.into(), new_account_state.into()); } @@ -37,9 +37,17 @@ pub async fn build_expected_block_header( }; // Compute created notes root - // FIXME: compute the right root. Needs - // https://github.com/0xPolygonMiden/crypto/issues/220#issuecomment-1823911017 - let new_created_notes_root = Digest::default(); + let created_notes: Vec<&NoteEnvelope> = + batches.iter().flat_map(TransactionBatch::created_notes).collect(); + let new_created_notes_root = { + let mut entries: Vec<(u64, Word)> = Vec::with_capacity(created_notes.len() * 2); + for (index, note) in created_notes.iter().enumerate() { + entries.push(((index * 2) as u64, note.note_id().into())); + entries.push(((index * 2) as u64 + 1, note.metadata().into())); + } + + SimpleSmt::::with_leaves(entries).unwrap().root() + }; // Compute new chain MMR root let new_chain_mmr_root = { @@ -58,7 +66,6 @@ pub async fn build_expected_block_header( new_account_root, // FIXME: FILL IN CORRECT NULLIFIER ROOT Digest::default(), - // FIXME: FILL IN CORRECT CREATED NOTES ROOT new_created_notes_root, Digest::default(), Digest::default(), diff --git a/block-producer/src/test_utils/store.rs b/block-producer/src/test_utils/store.rs index 532b8756d..e06739027 100644 --- a/block-producer/src/test_utils/store.rs +++ b/block-producer/src/test_utils/store.rs @@ -3,8 +3,8 @@ use std::collections::BTreeSet; use async_trait::async_trait; use miden_objects::{ crypto::merkle::{Mmr, SimpleSmt, Smt, ValuePath}, - notes::Nullifier, - BlockHeader, ACCOUNT_TREE_DEPTH, EMPTY_WORD, ONE, ZERO, + notes::{NoteEnvelope, Nullifier}, + BlockHeader, Word, ACCOUNT_TREE_DEPTH, EMPTY_WORD, NOTE_TREE_DEPTH, ONE, ZERO, }; use super::*; @@ -20,6 +20,7 @@ use crate::{ #[derive(Debug, Default)] pub struct MockStoreSuccessBuilder { accounts: Option>, + notes: Option>, produced_nullifiers: Option, chain_mmr: Option, block_num: Option, @@ -48,6 +49,25 @@ impl MockStoreSuccessBuilder { self } + pub fn initial_notes( + mut self, + notes: impl Iterator, + ) -> Self { + let notes_smt = { + let mut entries: Vec<(u64, Word)> = Vec::with_capacity(notes.size_hint().0 * 2); + for (index, note) in notes.enumerate() { + entries.push(((index * 2) as u64, note.note_id().into())); + entries.push(((index * 2) as u64 + 1, note.metadata().into())); + } + + SimpleSmt::::with_leaves(entries).unwrap() + }; + + self.notes = Some(notes_smt); + + self + } + pub fn initial_nullifiers( mut self, nullifiers: BTreeSet, @@ -83,7 +103,8 @@ impl MockStoreSuccessBuilder { } pub fn build(self) -> MockStoreSuccess { - let accounts_smt = self.accounts.unwrap_or(SimpleSmt::::new().unwrap()); + let accounts_smt = self.accounts.unwrap_or(SimpleSmt::new().unwrap()); + let notes_smt = self.notes.unwrap_or(SimpleSmt::new().unwrap()); let nullifiers_smt = self.produced_nullifiers.unwrap_or_default(); let chain_mmr = self.chain_mmr.unwrap_or_default(); @@ -93,8 +114,7 @@ impl MockStoreSuccessBuilder { chain_mmr.peaks(chain_mmr.forest()).unwrap().hash_peaks(), accounts_smt.root(), nullifiers_smt.root(), - // FIXME: FILL IN CORRECT VALUE - Digest::default(), + notes_smt.root(), Digest::default(), Digest::default(), ZERO, @@ -103,6 +123,7 @@ impl MockStoreSuccessBuilder { MockStoreSuccess { accounts: Arc::new(RwLock::new(accounts_smt)), + notes: Arc::new(RwLock::new(notes_smt)), 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)), @@ -115,6 +136,9 @@ pub struct MockStoreSuccess { /// Map account id -> account hash pub accounts: Arc>>, + /// Stores notes created + pub notes: Arc>>, + /// Stores the nullifiers of the notes that were consumed pub produced_nullifiers: Arc>, @@ -144,6 +168,7 @@ impl ApplyBlock for MockStoreSuccess { ) -> Result<(), ApplyBlockError> { // Intentionally, we take and hold both locks, to prevent calls to `get_tx_inputs()` from going through while we're updating the store's data structure let mut locked_accounts = self.accounts.write().await; + let mut locked_notes = self.notes.write().await; let mut locked_produced_nullifiers = self.produced_nullifiers.write().await; // update accounts @@ -152,6 +177,17 @@ impl ApplyBlock for MockStoreSuccess { } debug_assert_eq!(locked_accounts.root(), block.header.account_root()); + // update notes + let mut entries: Vec<(u64, Word)> = Vec::with_capacity(block.created_notes.len() * 2); + for (index, note) in block.created_notes.iter() { + entries.push((index * 2, note.note_id().into())); + entries.push((index * 2 + 1, note.metadata().into())); + } + let created_notes = SimpleSmt::::with_leaves(entries)?; + debug_assert_eq!(created_notes.root(), block.header.note_root()); + let new_notes_root = locked_notes.set_subtree(0, created_notes)?; + debug_assert_eq!(new_notes_root, block.header.note_root()); + // update nullifiers for nullifier in block.produced_nullifiers { locked_produced_nullifiers From bf98560e260f69ca3b05b6b4e71eb1851a6e1730 Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Tue, 27 Feb 2024 19:53:08 +0500 Subject: [PATCH 2/4] fix: using `BLOCK_OUTPUT_NOTES_TREE_DEPTH` as depth --- block-producer/src/test_utils/block.rs | 4 ++-- block-producer/src/test_utils/store.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/block-producer/src/test_utils/block.rs b/block-producer/src/test_utils/block.rs index 446e56b7e..551cf6f26 100644 --- a/block-producer/src/test_utils/block.rs +++ b/block-producer/src/test_utils/block.rs @@ -4,7 +4,7 @@ use miden_objects::{ accounts::AccountId, crypto::merkle::Mmr, notes::{NoteEnvelope, Nullifier}, - BlockHeader, Digest, Word, ACCOUNT_TREE_DEPTH, NOTE_TREE_DEPTH, ONE, ZERO, + BlockHeader, Digest, Word, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, ONE, ZERO, }; use miden_vm::crypto::SimpleSmt; @@ -46,7 +46,7 @@ pub async fn build_expected_block_header( entries.push(((index * 2) as u64 + 1, note.metadata().into())); } - SimpleSmt::::with_leaves(entries).unwrap().root() + SimpleSmt::::with_leaves(entries).unwrap().root() }; // Compute new chain MMR root diff --git a/block-producer/src/test_utils/store.rs b/block-producer/src/test_utils/store.rs index e06739027..59a6df46f 100644 --- a/block-producer/src/test_utils/store.rs +++ b/block-producer/src/test_utils/store.rs @@ -4,7 +4,7 @@ use async_trait::async_trait; use miden_objects::{ crypto::merkle::{Mmr, SimpleSmt, Smt, ValuePath}, notes::{NoteEnvelope, Nullifier}, - BlockHeader, Word, ACCOUNT_TREE_DEPTH, EMPTY_WORD, NOTE_TREE_DEPTH, ONE, ZERO, + BlockHeader, Word, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, EMPTY_WORD, ONE, ZERO, }; use super::*; @@ -20,7 +20,7 @@ use crate::{ #[derive(Debug, Default)] pub struct MockStoreSuccessBuilder { accounts: Option>, - notes: Option>, + notes: Option>, produced_nullifiers: Option, chain_mmr: Option, block_num: Option, @@ -60,7 +60,7 @@ impl MockStoreSuccessBuilder { entries.push(((index * 2) as u64 + 1, note.metadata().into())); } - SimpleSmt::::with_leaves(entries).unwrap() + SimpleSmt::::with_leaves(entries).unwrap() }; self.notes = Some(notes_smt); @@ -137,7 +137,7 @@ pub struct MockStoreSuccess { pub accounts: Arc>>, /// Stores notes created - pub notes: Arc>>, + pub notes: Arc>>, /// Stores the nullifiers of the notes that were consumed pub produced_nullifiers: Arc>, @@ -183,7 +183,7 @@ impl ApplyBlock for MockStoreSuccess { entries.push((index * 2, note.note_id().into())); entries.push((index * 2 + 1, note.metadata().into())); } - let created_notes = SimpleSmt::::with_leaves(entries)?; + let created_notes = SimpleSmt::::with_leaves(entries)?; debug_assert_eq!(created_notes.root(), block.header.note_root()); let new_notes_root = locked_notes.set_subtree(0, created_notes)?; debug_assert_eq!(new_notes_root, block.header.note_root()); From f323ffa15d240388a41bc68953b5995ba47ac68f Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Thu, 29 Feb 2024 16:56:29 +0500 Subject: [PATCH 3/4] feat: implemented `MockStoreSuccessBuilder::from_batches`, multiple improvements --- block-producer/src/batch_builder/batch.rs | 6 +- .../src/block_builder/prover/mod.rs | 2 +- .../src/block_builder/prover/tests.rs | 55 ++++---- block-producer/src/block_builder/tests.rs | 7 +- .../src/state_view/tests/apply_block.rs | 36 +++--- .../src/state_view/tests/verify_tx.rs | 55 ++++---- block-producer/src/test_utils/block.rs | 50 +++++--- block-producer/src/test_utils/proven_tx.rs | 3 +- block-producer/src/test_utils/store.rs | 117 +++++++++--------- store/src/state.rs | 2 +- 10 files changed, 168 insertions(+), 165 deletions(-) diff --git a/block-producer/src/batch_builder/batch.rs b/block-producer/src/batch_builder/batch.rs index 82ef42667..2f4752f1d 100644 --- a/block-producer/src/batch_builder/batch.rs +++ b/block-producer/src/batch_builder/batch.rs @@ -116,17 +116,17 @@ impl TransactionBatch { .map(|(account_id, account_states)| (*account_id, account_states.final_state)) } - /// Returns the nullifier of all consumed notes. + /// Returns an iterator over produced nullifiers for all consumed notes. pub fn produced_nullifiers(&self) -> impl Iterator + '_ { self.produced_nullifiers.iter().cloned() } - /// Returns the hash of created notes. + /// Returns an iterator over created notes. pub fn created_notes(&self) -> impl Iterator + '_ { self.created_notes.iter() } - /// Returns the root of the created notes SMT. + /// Returns the root hash of the created notes SMT. pub fn created_notes_root(&self) -> Digest { self.created_notes_smt.root() } diff --git a/block-producer/src/block_builder/prover/mod.rs b/block-producer/src/block_builder/prover/mod.rs index 0e2f02b7e..cf5a7462f 100644 --- a/block-producer/src/block_builder/prover/mod.rs +++ b/block-producer/src/block_builder/prover/mod.rs @@ -224,7 +224,7 @@ impl BlockProver { let proof_hash = Digest::default(); let timestamp: Felt = SystemTime::now() .duration_since(UNIX_EPOCH) - .expect("today is expected to be before 1970") + .expect("today is expected to be after 1970") .as_millis() .try_into() .expect("timestamp is greater than or equal to the field modulus"); diff --git a/block-producer/src/block_builder/prover/tests.rs b/block-producer/src/block_builder/prover/tests.rs index 76931da12..581274bec 100644 --- a/block-producer/src/block_builder/prover/tests.rs +++ b/block-producer/src/block_builder/prover/tests.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::{collections::BTreeMap, iter}; use miden_mock::mock::block::mock_block_header; use miden_objects::{ @@ -188,14 +188,13 @@ async fn test_compute_account_root_success() { // Set up store's account SMT // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new() - .initial_accounts( - account_ids - .iter() - .zip(account_initial_states.iter()) - .map(|(&account_id, &account_hash)| (account_id, account_hash.into())), - ) - .build(); + let store = MockStoreSuccessBuilder::from_accounts( + account_ids + .iter() + .zip(account_initial_states.iter()) + .map(|(&account_id, &account_hash)| (account_id, account_hash.into())), + ) + .build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -272,14 +271,13 @@ async fn test_compute_account_root_empty_batches() { // Set up store's account SMT // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new() - .initial_accounts( - account_ids - .iter() - .zip(account_initial_states.iter()) - .map(|(&account_id, &account_hash)| (account_id, account_hash.into())), - ) - .build(); + let store = MockStoreSuccessBuilder::from_accounts( + account_ids + .iter() + .zip(account_initial_states.iter()) + .map(|(&account_id, &account_hash)| (account_id, account_hash.into())), + ) + .build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -310,7 +308,7 @@ async fn test_compute_note_root_empty_batches_success() { // Set up store // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new().build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()).build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -340,7 +338,7 @@ async fn test_compute_note_root_empty_notes_success() { // Set up store // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new().build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()).build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -391,7 +389,7 @@ async fn test_compute_note_root_success() { // Set up store // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new().build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()).build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -562,9 +560,7 @@ async fn test_compute_nullifier_root_empty_success() { // Set up store // --------------------------------------------------------------------------------------------- - let store = MockStoreSuccessBuilder::new() - .initial_accounts(batches.iter().flat_map(|batch| batch.account_initial_states())) - .build(); + let store = MockStoreSuccessBuilder::from_batches(batches.iter()).build(); // Block prover // --------------------------------------------------------------------------------------------- @@ -621,8 +617,7 @@ async fn test_compute_nullifier_root_success() { // --------------------------------------------------------------------------------------------- let initial_block_num = 42; - let store = MockStoreSuccessBuilder::new() - .initial_accounts(batches.iter().flat_map(|batch| batch.account_initial_states())) + let store = MockStoreSuccessBuilder::from_batches(batches.iter()) .initial_block_num(initial_block_num) .build(); @@ -660,7 +655,7 @@ async fn test_compute_nullifier_root_success() { #[tokio::test] #[miden_node_test_macro::enable_logging] async fn test_compute_chain_mmr_root_empty_mmr() { - let store = MockStoreSuccessBuilder::new().build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()).build(); let expected_block_header = build_expected_block_header(&store, &[]).await; let actual_block_header = build_actual_block_header(&store, Vec::new()).await; @@ -679,7 +674,9 @@ async fn test_compute_chain_mmr_root_mmr_1_peak() { mmr }; - let store = MockStoreSuccessBuilder::new().initial_chain_mmr(initial_chain_mmr).build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()) + .initial_chain_mmr(initial_chain_mmr) + .build(); let expected_block_header = build_expected_block_header(&store, &[]).await; let actual_block_header = build_actual_block_header(&store, Vec::new()).await; @@ -702,7 +699,9 @@ async fn test_compute_chain_mmr_root_mmr_17_peaks() { mmr }; - let store = MockStoreSuccessBuilder::new().initial_chain_mmr(initial_chain_mmr).build(); + let store = MockStoreSuccessBuilder::from_batches(iter::empty()) + .initial_chain_mmr(initial_chain_mmr) + .build(); let expected_block_header = build_expected_block_header(&store, &[]).await; let actual_block_header = build_actual_block_header(&store, Vec::new()).await; diff --git a/block-producer/src/block_builder/tests.rs b/block-producer/src/block_builder/tests.rs index 47eda8ddb..2b34f1da8 100644 --- a/block-producer/src/block_builder/tests.rs +++ b/block-producer/src/block_builder/tests.rs @@ -13,8 +13,7 @@ async fn test_apply_block_called_nonempty_batches() { let account_initial_hash: Digest = [Felt::new(1u64), Felt::new(1u64), Felt::new(1u64), Felt::new(1u64)].into(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(std::iter::once((account_id, account_initial_hash))) + MockStoreSuccessBuilder::from_accounts(std::iter::once((account_id, account_initial_hash))) .build(), ); @@ -48,9 +47,7 @@ async fn test_apply_block_called_empty_batches() { let account_hash: Digest = [Felt::new(1u64), Felt::new(1u64), Felt::new(1u64), Felt::new(1u64)].into(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(std::iter::once((account_id, account_hash))) - .build(), + MockStoreSuccessBuilder::from_accounts(std::iter::once((account_id, account_hash))).build(), ); let block_builder = DefaultBlockBuilder::new(store.clone(), store.clone()); diff --git a/block-producer/src/state_view/tests/apply_block.rs b/block-producer/src/state_view/tests/apply_block.rs index e3183bf90..ac4a52348 100644 --- a/block-producer/src/state_view/tests/apply_block.rs +++ b/block-producer/src/state_view/tests/apply_block.rs @@ -16,9 +16,7 @@ async fn test_apply_block_ab1() { let account: MockPrivateAccount<3> = MockPrivateAccount::from(0); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(iter::once((account.id, account.states[0]))) - .build(), + MockStoreSuccessBuilder::from_accounts(iter::once((account.id, account.states[0]))).build(), ); let tx = @@ -52,14 +50,13 @@ async fn test_apply_block_ab2() { let (txs, accounts): (Vec<_>, Vec<_>) = get_txs_and_accounts(0, 3).unzip(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts( - accounts - .clone() - .into_iter() - .map(|mock_account| (mock_account.id, mock_account.states[0])), - ) - .build(), + MockStoreSuccessBuilder::from_accounts( + accounts + .clone() + .into_iter() + .map(|mock_account| (mock_account.id, mock_account.states[0])), + ) + .build(), ); let state_view = DefaultStateView::new(store.clone(), false); @@ -100,14 +97,13 @@ async fn test_apply_block_ab3() { let (txs, accounts): (Vec<_>, Vec<_>) = get_txs_and_accounts(0, 3).unzip(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts( - accounts - .clone() - .into_iter() - .map(|mock_account| (mock_account.id, mock_account.states[0])), - ) - .build(), + MockStoreSuccessBuilder::from_accounts( + accounts + .clone() + .into_iter() + .map(|mock_account| (mock_account.id, mock_account.states[0])), + ) + .build(), ); let state_view = DefaultStateView::new(store.clone(), false); @@ -132,7 +128,7 @@ async fn test_apply_block_ab3() { let apply_block_res = state_view.apply_block(block).await; assert!(apply_block_res.is_ok()); - // Craft a new transaction which tries to consume the same note that was consumed in in the + // Craft a new transaction which tries to consume the same note that was consumed in the // first tx let tx_new = MockProvenTxBuilder::with_account( accounts[0].id, diff --git a/block-producer/src/state_view/tests/verify_tx.rs b/block-producer/src/state_view/tests/verify_tx.rs index 1924c6f0e..1dcea439c 100644 --- a/block-producer/src/state_view/tests/verify_tx.rs +++ b/block-producer/src/state_view/tests/verify_tx.rs @@ -26,13 +26,12 @@ async fn test_verify_tx_happy_path() { get_txs_and_accounts(0, 3).unzip(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts( - accounts - .into_iter() - .map(|mock_account| (mock_account.id, mock_account.states[0])), - ) - .build(), + MockStoreSuccessBuilder::from_accounts( + accounts + .into_iter() + .map(|mock_account| (mock_account.id, mock_account.states[0])), + ) + .build(), ); let state_view = DefaultStateView::new(store, false); @@ -53,13 +52,12 @@ async fn test_verify_tx_happy_path_concurrent() { get_txs_and_accounts(0, 3).unzip(); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts( - accounts - .into_iter() - .map(|mock_account| (mock_account.id, mock_account.states[0])), - ) - .build(), + MockStoreSuccessBuilder::from_accounts( + accounts + .into_iter() + .map(|mock_account| (mock_account.id, mock_account.states[0])), + ) + .build(), ); let state_view = Arc::new(DefaultStateView::new(store, false)); @@ -83,9 +81,7 @@ async fn test_verify_tx_vt1() { let account = MockPrivateAccount::<3>::from(1); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(iter::once((account.id, account.states[0]))) - .build(), + MockStoreSuccessBuilder::from_accounts(iter::once((account.id, account.states[0]))).build(), ); // The transaction's initial account hash uses `account.states[1]`, where the store expects @@ -114,7 +110,7 @@ async fn test_verify_tx_vt2() { let account_not_in_store: MockPrivateAccount<3> = MockPrivateAccount::from(0); // Notice: account is not added to the store - let store = Arc::new(MockStoreSuccessBuilder::new().build()); + let store = Arc::new(MockStoreSuccessBuilder::from_batches(iter::empty()).build()); let tx = MockProvenTxBuilder::with_account( account_not_in_store.id, @@ -141,9 +137,9 @@ async fn test_verify_tx_vt3() { // Notice: `consumed_note_in_store` is added to the store let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(iter::once((account.id, account.states[0]))) - .initial_nullifiers(BTreeSet::from_iter(iter::once(nullifier_in_store.inner())), 1) + MockStoreSuccessBuilder::from_accounts(iter::once((account.id, account.states[0]))) + .initial_nullifiers(BTreeSet::from_iter(iter::once(nullifier_in_store.inner()))) + .initial_block_num(1) .build(), ); @@ -170,9 +166,7 @@ async fn test_verify_tx_vt4() { let account: MockPrivateAccount<3> = MockPrivateAccount::from(1); let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts(iter::once((account.id, account.states[0]))) - .build(), + MockStoreSuccessBuilder::from_accounts(iter::once((account.id, account.states[0]))).build(), ); let tx1 = @@ -205,13 +199,12 @@ async fn test_verify_tx_vt5() { // Notice: `consumed_note_in_both_txs` is NOT in the store let store = Arc::new( - MockStoreSuccessBuilder::new() - .initial_accounts( - vec![account_1, account_2] - .into_iter() - .map(|account| (account.id, account.states[0])), - ) - .build(), + MockStoreSuccessBuilder::from_accounts( + vec![account_1, account_2] + .into_iter() + .map(|account| (account.id, account.states[0])), + ) + .build(), ); let tx1 = diff --git a/block-producer/src/test_utils/block.rs b/block-producer/src/test_utils/block.rs index 551cf6f26..02807b6cf 100644 --- a/block-producer/src/test_utils/block.rs +++ b/block-producer/src/test_utils/block.rs @@ -4,7 +4,8 @@ use miden_objects::{ accounts::AccountId, crypto::merkle::Mmr, notes::{NoteEnvelope, Nullifier}, - BlockHeader, Digest, Word, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, ONE, ZERO, + BlockHeader, Digest, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, MAX_NOTES_PER_BATCH, + ONE, ZERO, }; use miden_vm::crypto::SimpleSmt; @@ -36,19 +37,6 @@ pub async fn build_expected_block_header( store_accounts.root() }; - // Compute created notes root - let created_notes: Vec<&NoteEnvelope> = - batches.iter().flat_map(TransactionBatch::created_notes).collect(); - let new_created_notes_root = { - let mut entries: Vec<(u64, Word)> = Vec::with_capacity(created_notes.len() * 2); - for (index, note) in created_notes.iter().enumerate() { - entries.push(((index * 2) as u64, note.note_id().into())); - entries.push(((index * 2) as u64 + 1, note.metadata().into())); - } - - SimpleSmt::::with_leaves(entries).unwrap().root() - }; - // Compute new chain MMR root let new_chain_mmr_root = { let mut store_chain_mmr = store.chain_mmr.read().await.clone(); @@ -66,7 +54,7 @@ pub async fn build_expected_block_header( new_account_root, // FIXME: FILL IN CORRECT NULLIFIER ROOT Digest::default(), - new_created_notes_root, + note_created_smt_from_batches(batches.iter()).root(), Digest::default(), Digest::default(), ZERO, @@ -154,13 +142,15 @@ impl MockBlockBuilder { } pub fn build(self) -> Block { + let created_notes = self.created_notes.unwrap_or_default(); + let header = BlockHeader::new( self.last_block_header.hash(), self.last_block_header.block_num() + 1, self.store_chain_mmr.peaks(self.store_chain_mmr.forest()).unwrap().hash_peaks(), self.store_accounts.root(), Digest::default(), - Digest::default(), + note_created_smt_from_envelopes(created_notes.iter()).root(), Digest::default(), Digest::default(), ZERO, @@ -170,8 +160,34 @@ impl MockBlockBuilder { Block { header, updated_accounts: self.updated_accounts.unwrap_or_default(), - created_notes: self.created_notes.unwrap_or_default(), + created_notes, produced_nullifiers: self.produced_nullifiers.unwrap_or_default(), } } } + +pub(crate) fn note_created_smt_from_envelopes<'a>( + note_iterator: impl Iterator +) -> SimpleSmt { + SimpleSmt::::with_leaves(note_iterator.flat_map( + |(note_idx_in_block, note)| { + let index = note_idx_in_block * 2; + [(index, note.note_id().into()), (index + 1, note.metadata().into())] + }, + )) + .unwrap() +} + +pub(crate) fn note_created_smt_from_batches<'a>( + batches: impl Iterator +) -> SimpleSmt { + let note_leaf_iterator = batches.enumerate().flat_map(|(batch_index, batch)| { + let subtree_index = batch_index * MAX_NOTES_PER_BATCH * 2; + batch.created_notes().enumerate().flat_map(move |(note_index, note)| { + let index = (subtree_index + note_index * 2) as u64; + [(index, note.note_id().into()), (index + 1, note.metadata().into())] + }) + }); + + SimpleSmt::::with_leaves(note_leaf_iterator).unwrap() +} diff --git a/block-producer/src/test_utils/proven_tx.rs b/block-producer/src/test_utils/proven_tx.rs index 0d57d7c59..890fc23f4 100644 --- a/block-producer/src/test_utils/proven_tx.rs +++ b/block-producer/src/test_utils/proven_tx.rs @@ -64,8 +64,7 @@ impl MockProvenTxBuilder { ) -> Self { let nullifiers = range .map(|index| { - let nullifier = - Digest::from([Felt::new(1), Felt::new(1), Felt::new(1), Felt::new(index)]); + let nullifier = Digest::from([ONE, ONE, ONE, Felt::new(index)]); Nullifier::from(nullifier) }) diff --git a/block-producer/src/test_utils/store.rs b/block-producer/src/test_utils/store.rs index 59a6df46f..912237761 100644 --- a/block-producer/src/test_utils/store.rs +++ b/block-producer/src/test_utils/store.rs @@ -4,66 +4,81 @@ use async_trait::async_trait; use miden_objects::{ crypto::merkle::{Mmr, SimpleSmt, Smt, ValuePath}, notes::{NoteEnvelope, Nullifier}, - BlockHeader, Word, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, EMPTY_WORD, ONE, ZERO, + BlockHeader, ACCOUNT_TREE_DEPTH, BLOCK_OUTPUT_NOTES_TREE_DEPTH, EMPTY_WORD, ONE, ZERO, }; use super::*; use crate::{ + batch_builder::TransactionBatch, block::{AccountWitness, Block, BlockInputs}, store::{ ApplyBlock, ApplyBlockError, BlockInputsError, Store, TransactionInputs, TxInputsError, }, + test_utils::block::{note_created_smt_from_batches, note_created_smt_from_envelopes}, ProvenTransaction, }; /// Builds a [`MockStoreSuccess`] -#[derive(Debug, Default)] +#[derive(Debug)] pub struct MockStoreSuccessBuilder { accounts: Option>, notes: Option>, - produced_nullifiers: Option, + produced_nullifiers: Option>, chain_mmr: Option, block_num: Option, } impl MockStoreSuccessBuilder { - /// FIXME: the store always needs to be properly initialized with initial accounts - /// see https://github.com/0xPolygonMiden/miden-node/issues/79 - pub fn new() -> Self { - Self::default() - } + pub fn from_batches<'a>(batches: impl Iterator) -> Self { + let batches: Vec<_> = batches.collect(); - pub fn initial_accounts( - mut self, - accounts: impl Iterator, - ) -> Self { let accounts_smt = { - let accounts = - accounts.into_iter().map(|(account_id, hash)| (account_id.into(), hash.into())); - + let accounts = batches + .iter() + .cloned() + .flat_map(TransactionBatch::account_initial_states) + .map(|(account_id, hash)| (account_id.into(), hash.into())); SimpleSmt::::with_leaves(accounts).unwrap() }; - self.accounts = Some(accounts_smt); + let created_notes = note_created_smt_from_batches(batches.iter().cloned()); + let produced_nullifiers = batches + .iter() + .cloned() + .flat_map(TransactionBatch::produced_nullifiers) + .map(|nullifier| nullifier.inner()) + .collect(); - self + Self { + accounts: Some(accounts_smt), + notes: Some(created_notes), + produced_nullifiers: Some(produced_nullifiers), + chain_mmr: None, + block_num: None, + } } - pub fn initial_notes( - mut self, - notes: impl Iterator, - ) -> Self { - let notes_smt = { - let mut entries: Vec<(u64, Word)> = Vec::with_capacity(notes.size_hint().0 * 2); - for (index, note) in notes.enumerate() { - entries.push(((index * 2) as u64, note.note_id().into())); - entries.push(((index * 2) as u64 + 1, note.metadata().into())); - } + pub fn from_accounts(accounts: impl Iterator) -> Self { + let accounts_smt = { + let accounts = accounts.map(|(account_id, hash)| (account_id.into(), hash.into())); - SimpleSmt::::with_leaves(entries).unwrap() + SimpleSmt::::with_leaves(accounts).unwrap() }; - self.notes = Some(notes_smt); + Self { + accounts: Some(accounts_smt), + notes: None, + produced_nullifiers: None, + chain_mmr: None, + block_num: None, + } + } + + pub fn initial_notes<'a>( + mut self, + notes: impl Iterator, + ) -> Self { + self.notes = Some(note_created_smt_from_envelopes(notes)); self } @@ -71,15 +86,8 @@ impl MockStoreSuccessBuilder { pub fn initial_nullifiers( mut self, nullifiers: BTreeSet, - block_num: u32, ) -> Self { - let smt = Smt::with_entries( - nullifiers - .into_iter() - .map(|nullifier| (nullifier, [ZERO, ZERO, ZERO, block_num.into()])), - ) - .unwrap(); - self.produced_nullifiers = Some(smt); + self.produced_nullifiers = Some(nullifiers); self } @@ -103,14 +111,25 @@ impl MockStoreSuccessBuilder { } 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(SimpleSmt::new().unwrap()); - let nullifiers_smt = self.produced_nullifiers.unwrap_or_default(); let chain_mmr = self.chain_mmr.unwrap_or_default(); + let nullifiers_smt = self + .produced_nullifiers + .map(|nullifiers| { + Smt::with_entries( + nullifiers + .into_iter() + .map(|nullifier| (nullifier, [block_num.into(), ZERO, ZERO, ZERO])), + ) + .unwrap() + }) + .unwrap_or_default(); let initial_block_header = BlockHeader::new( Digest::default(), - self.block_num.unwrap_or(1), + block_num, chain_mmr.peaks(chain_mmr.forest()).unwrap().hash_peaks(), accounts_smt.root(), nullifiers_smt.root(), @@ -123,7 +142,6 @@ impl MockStoreSuccessBuilder { MockStoreSuccess { accounts: Arc::new(RwLock::new(accounts_smt)), - notes: Arc::new(RwLock::new(notes_smt)), 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)), @@ -136,9 +154,6 @@ pub struct MockStoreSuccess { /// Map account id -> account hash pub accounts: Arc>>, - /// Stores notes created - pub notes: Arc>>, - /// Stores the nullifiers of the notes that were consumed pub produced_nullifiers: Arc>, @@ -168,7 +183,6 @@ impl ApplyBlock for MockStoreSuccess { ) -> Result<(), ApplyBlockError> { // Intentionally, we take and hold both locks, to prevent calls to `get_tx_inputs()` from going through while we're updating the store's data structure let mut locked_accounts = self.accounts.write().await; - let mut locked_notes = self.notes.write().await; let mut locked_produced_nullifiers = self.produced_nullifiers.write().await; // update accounts @@ -177,21 +191,10 @@ impl ApplyBlock for MockStoreSuccess { } debug_assert_eq!(locked_accounts.root(), block.header.account_root()); - // update notes - let mut entries: Vec<(u64, Word)> = Vec::with_capacity(block.created_notes.len() * 2); - for (index, note) in block.created_notes.iter() { - entries.push((index * 2, note.note_id().into())); - entries.push((index * 2 + 1, note.metadata().into())); - } - let created_notes = SimpleSmt::::with_leaves(entries)?; - debug_assert_eq!(created_notes.root(), block.header.note_root()); - let new_notes_root = locked_notes.set_subtree(0, created_notes)?; - debug_assert_eq!(new_notes_root, block.header.note_root()); - // update nullifiers for nullifier in block.produced_nullifiers { locked_produced_nullifiers - .insert(nullifier.inner(), [ZERO, ZERO, ZERO, block.header.block_num().into()]); + .insert(nullifier.inner(), [block.header.block_num().into(), ZERO, ZERO, ZERO]); } // update chain mmr with new block header hash @@ -236,7 +239,7 @@ impl Store for MockStoreSuccess { .map(|nullifier| { let nullifier_value = locked_produced_nullifiers.get_value(&nullifier.inner()); - (*nullifier, nullifier_value[3].inner() as u32) + (*nullifier, nullifier_value[0].inner() as u32) }) .collect(); diff --git a/store/src/state.rs b/store/src/state.rs index 98c9014bd..cc4588425 100644 --- a/store/src/state.rs +++ b/store/src/state.rs @@ -478,7 +478,7 @@ impl State { // UTILITIES // ================================================================================================ -/// Returns the nullifier's block number given its leaf value in the SMT. +/// Returns the nullifier's leaf value in the SMT by its block number. fn block_to_nullifier_data(block: BlockNumber) -> Word { [Felt::from(block), Felt::ZERO, Felt::ZERO, Felt::ZERO] } From f1205e5058706799310cd795a67931ebb8672f81 Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Fri, 1 Mar 2024 18:06:26 +0500 Subject: [PATCH 4/4] fix: `MockStoreSuccessBuilder::from_batches` --- block-producer/src/test_utils/store.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/block-producer/src/test_utils/store.rs b/block-producer/src/test_utils/store.rs index 912237761..00f26a2a1 100644 --- a/block-producer/src/test_utils/store.rs +++ b/block-producer/src/test_utils/store.rs @@ -42,17 +42,11 @@ impl MockStoreSuccessBuilder { }; let created_notes = note_created_smt_from_batches(batches.iter().cloned()); - let produced_nullifiers = batches - .iter() - .cloned() - .flat_map(TransactionBatch::produced_nullifiers) - .map(|nullifier| nullifier.inner()) - .collect(); Self { accounts: Some(accounts_smt), notes: Some(created_notes), - produced_nullifiers: Some(produced_nullifiers), + produced_nullifiers: None, chain_mmr: None, block_num: None, }