From 79f75877d42a6a4214c494442c82563c58096f50 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Thu, 20 Feb 2025 15:19:50 +0100 Subject: [PATCH 01/10] feat: Allow empty proposed blocks --- .../src/tests/proposed_block_errors.rs | 19 ------------- .../src/tests/proposed_block_success.rs | 28 +++++++++++++++++-- .../miden-objects/src/block/proposed_block.rs | 8 ++---- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/crates/miden-block-prover/src/tests/proposed_block_errors.rs b/crates/miden-block-prover/src/tests/proposed_block_errors.rs index 661fb31f6..e5d395855 100644 --- a/crates/miden-block-prover/src/tests/proposed_block_errors.rs +++ b/crates/miden-block-prover/src/tests/proposed_block_errors.rs @@ -18,25 +18,6 @@ use crate::tests::utils::{ generate_untracked_note_with_output_note, setup_chain, ProvenTransactionExt, TestSetup, }; -/// Tests that empty batches produce an error. -#[test] -fn proposed_block_fails_on_empty_batches() -> anyhow::Result<()> { - let TestSetup { chain, .. } = setup_chain(2); - - let block_inputs = BlockInputs::new( - chain.latest_block_header(), - chain.latest_chain_mmr(), - BTreeMap::default(), - BTreeMap::default(), - BTreeMap::default(), - ); - let error = ProposedBlock::new(block_inputs, Vec::new()).unwrap_err(); - - assert_matches!(error, ProposedBlockError::EmptyBlock); - - Ok(()) -} - /// Tests that too many batches produce an error. #[test] fn proposed_block_fails_on_too_many_batches() -> anyhow::Result<()> { diff --git a/crates/miden-block-prover/src/tests/proposed_block_success.rs b/crates/miden-block-prover/src/tests/proposed_block_success.rs index 5b7c615a2..b30249592 100644 --- a/crates/miden-block-prover/src/tests/proposed_block_success.rs +++ b/crates/miden-block-prover/src/tests/proposed_block_success.rs @@ -2,8 +2,10 @@ use std::{collections::BTreeMap, vec::Vec}; use anyhow::Context; use miden_objects::{ - account::AccountId, block::ProposedBlock, - testing::account_id::ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, transaction::ProvenTransaction, + account::AccountId, + block::{BlockInputs, ProposedBlock}, + testing::account_id::ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, + transaction::ProvenTransaction, }; use crate::tests::utils::{ @@ -13,6 +15,28 @@ use crate::tests::utils::{ ProvenTransactionExt, TestSetup, }; +/// Tests that we can build empty blocks. +#[test] +fn proposed_block_succeeds_with_empty_batches() -> anyhow::Result<()> { + let TestSetup { chain, .. } = setup_chain(2); + + let block_inputs = BlockInputs::new( + chain.latest_block_header(), + chain.latest_chain_mmr(), + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + ); + let block = ProposedBlock::new(block_inputs, Vec::new()).context("failed to propose block")?; + + assert_eq!(block.affected_accounts().count(), 0); + assert_eq!(block.output_note_batches().len(), 0); + assert_eq!(block.nullifiers().len(), 0); + assert_eq!(block.batches().len(), 0); + + Ok(()) +} + /// Tests that a proposed block from two batches with one transaction each can be successfully /// built. #[test] diff --git a/crates/miden-objects/src/block/proposed_block.rs b/crates/miden-objects/src/block/proposed_block.rs index 05276935b..725bb5209 100644 --- a/crates/miden-objects/src/block/proposed_block.rs +++ b/crates/miden-objects/src/block/proposed_block.rs @@ -77,7 +77,7 @@ impl ProposedBlock { /// /// ## Batches /// - /// - The number of batches is zero or exceeds [`MAX_BATCHES_PER_BLOCK`]. + /// - The number of batches exceeds [`MAX_BATCHES_PER_BLOCK`]. /// - There are duplicate batches, i.e. they have the same [`BatchId`]. /// - The expiration block number of any batch is less than the block number of the currently /// proposed block. @@ -130,13 +130,9 @@ impl ProposedBlock { batches: Vec, timestamp: u32, ) -> Result { - // Check for empty or duplicate batches. + // Check for duplicate and max number of batches. // -------------------------------------------------------------------------------------------- - if batches.is_empty() { - return Err(ProposedBlockError::EmptyBlock); - } - if batches.len() > MAX_BATCHES_PER_BLOCK { return Err(ProposedBlockError::TooManyBatches); } From 5242b768b2582fb0db58086634dfaab09e557b1d Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Thu, 20 Feb 2025 17:13:50 +0100 Subject: [PATCH 02/10] feat: Use previous account/nullifier roots if block is empty --- .../src/local_block_prover.rs | 10 +++ .../src/tests/proven_block_success.rs | 87 ++++++++++++++++++- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/crates/miden-block-prover/src/local_block_prover.rs b/crates/miden-block-prover/src/local_block_prover.rs index 58277d366..05cb15c8e 100644 --- a/crates/miden-block-prover/src/local_block_prover.rs +++ b/crates/miden-block-prover/src/local_block_prover.rs @@ -182,6 +182,11 @@ fn compute_nullifiers( prev_block_header: &BlockHeader, block_num: BlockNumber, ) -> Result<(Vec, Digest), ProvenBlockError> { + // If no nullifiers were created, the nullifier tree root is unchanged. + if created_nullifiers.is_empty() { + return Ok((Vec::new(), prev_block_header.nullifier_root())); + } + let nullifiers: Vec = created_nullifiers.keys().copied().collect(); let mut partial_nullifier_tree = PartialNullifierTree::new(); @@ -234,6 +239,11 @@ fn compute_account_root( updated_accounts: &mut Vec<(AccountId, AccountUpdateWitness)>, prev_block_header: &BlockHeader, ) -> Result { + // If no accounts were updated, the account tree root is unchanged. + if updated_accounts.is_empty() { + return Ok(prev_block_header.account_root()); + } + let mut partial_account_tree = PartialMerkleTree::new(); // First reconstruct the current account tree from the provided merkle paths. diff --git a/crates/miden-block-prover/src/tests/proven_block_success.rs b/crates/miden-block-prover/src/tests/proven_block_success.rs index d82e8c5b8..7572df806 100644 --- a/crates/miden-block-prover/src/tests/proven_block_success.rs +++ b/crates/miden-block-prover/src/tests/proven_block_success.rs @@ -1,18 +1,19 @@ use std::{collections::BTreeMap, vec::Vec}; use anyhow::Context; -use miden_crypto::merkle::LeafIndex; +use miden_crypto::merkle::{LeafIndex, SimpleSmt, Smt}; use miden_objects::{ batch::BatchNoteTree, - block::{BlockNoteIndex, BlockNoteTree, ProposedBlock}, + block::{BlockInputs, BlockNoteIndex, BlockNoteTree, ProposedBlock}, transaction::InputNoteCommitment, - Felt, FieldElement, MIN_PROOF_SECURITY_LEVEL, + Felt, FieldElement, ACCOUNT_TREE_DEPTH, MIN_PROOF_SECURITY_LEVEL, }; use rand::Rng; use crate::{ tests::utils::{ - generate_batch, generate_output_note, generate_tx_with_authenticated_notes, + generate_batch, generate_executed_tx_with_authenticated_notes, generate_output_note, + generate_tracked_note, generate_tx_with_authenticated_notes, generate_tx_with_unauthenticated_notes, generate_untracked_note_with_output_note, setup_chain, TestSetup, }, @@ -324,3 +325,81 @@ fn proven_block_erasing_unauthenticated_notes() -> anyhow::Result<()> { Ok(()) } + +/// Tests that we can build empty blocks. +#[test] +fn proven_block_succeeds_with_empty_batches() -> anyhow::Result<()> { + // Setup a chain with a non-empty nullifier tree by consuming some notes. + // -------------------------------------------------------------------------------------------- + + let TestSetup { mut chain, mut accounts, .. } = setup_chain(2); + + let account0 = accounts.remove(&0).unwrap(); + let account1 = accounts.remove(&1).unwrap(); + + // Add notes to the chain we can consume. + let note0 = generate_tracked_note(&mut chain, account1.id(), account0.id()); + let note1 = generate_tracked_note(&mut chain, account0.id(), account1.id()); + chain.seal_block(None); + + let tx0 = + generate_executed_tx_with_authenticated_notes(&mut chain, account0.id(), &[note0.id()]); + let tx1 = + generate_executed_tx_with_authenticated_notes(&mut chain, account1.id(), &[note1.id()]); + + chain.apply_executed_transaction(&tx0); + chain.apply_executed_transaction(&tx1); + let blockx = chain.seal_block(None); + + // Build a block with empty inputs whose account tree and nullifier tree root are not the empty + // roots. + // If they are the empty roots, we do not run the branches of code that handle empty blocks. + // -------------------------------------------------------------------------------------------- + + let latest_block_header = chain.latest_block_header(); + assert_eq!(latest_block_header.hash(), blockx.hash()); + + // Sanity check: The account and nullifier tree roots should not be the empty tree roots. + assert_ne!( + latest_block_header.account_root(), + SimpleSmt::::new().unwrap().root() + ); + assert_ne!(latest_block_header.nullifier_root(), Smt::new().root()); + + let (_, empty_chain_mmr) = chain.latest_selective_chain_mmr([]); + assert_eq!(empty_chain_mmr.block_headers().count(), 0); + + let block_inputs = BlockInputs::new( + latest_block_header, + empty_chain_mmr.clone(), + BTreeMap::default(), + BTreeMap::default(), + BTreeMap::default(), + ); + + let proposed_block = + ProposedBlock::new(block_inputs, Vec::new()).context("failed to propose block")?; + + let proven_block = LocalBlockProver::new(MIN_PROOF_SECURITY_LEVEL) + .prove_without_batch_verification(proposed_block) + .context("failed to prove proposed block")?; + + // Nothing should be created or updated. + assert_eq!(proven_block.updated_accounts().len(), 0); + assert_eq!(proven_block.output_note_batches().len(), 0); + assert_eq!(proven_block.created_nullifiers().len(), 0); + assert!(proven_block.build_output_note_tree().is_empty()); + + // Account and nullifier root should match the previous block header's roots, since nothing has + // changed. + assert_eq!(proven_block.header().account_root(), latest_block_header.account_root()); + assert_eq!(proven_block.header().nullifier_root(), latest_block_header.nullifier_root()); + // Block note tree should be the empty root. + assert_eq!(proven_block.header().note_root(), BlockNoteTree::empty().root()); + + // The previous block header should have been added to the chain. + assert_eq!(proven_block.header().chain_root(), chain.block_chain().peaks().hash_peaks()); + assert_eq!(proven_block.header().block_num(), latest_block_header.block_num() + 1); + + Ok(()) +} From 8d936136dae03bf9504a9d8426aef2dadb90d641 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Thu, 20 Feb 2025 17:14:45 +0100 Subject: [PATCH 03/10] chore: Add changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d221be0cf..1906f7b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - Added serialization for `ProposedBatch`, `BatchId`, `BatchNoteTree` and `ProvenBatch` (#1140). - Added `prefix` to `Nullifier` (#1153). - [BREAKING] Implemented a `RemoteBatchProver`. `miden-proving-service` workers can prove batches (#1142). -- [BREAKING] Implement `LocalBlockProver` and rename `Block` to `ProvenBlock` (#1152, #1168). +- [BREAKING] Implement `LocalBlockProver` and rename `Block` to `ProvenBlock` (#1152, #1168, #1172). - [BREAKING] Added native types to `AccountComponentTemplate` (#1124). - Implemented `RemoteBlockProver`. `miden-proving-service` workers can prove blocks (#1169). From a95109582d986bfd1d2860ef26d824bf09eb8d05 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 08:14:42 +0100 Subject: [PATCH 04/10] feat: Explain why batch reference block proving is skipped --- .../miden-objects/src/batch/proposed_batch.rs | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/crates/miden-objects/src/batch/proposed_batch.rs b/crates/miden-objects/src/batch/proposed_batch.rs index 60b517c87..a28a42cdc 100644 --- a/crates/miden-objects/src/batch/proposed_batch.rs +++ b/crates/miden-objects/src/batch/proposed_batch.rs @@ -25,7 +25,7 @@ pub struct ProposedBatch { /// The transactions of this batch. transactions: Vec>, /// The header is boxed as it has a large stack size. - block_header: BlockHeader, + reference_block_header: BlockHeader, /// The chain MMR used to authenticate: /// - all unauthenticated notes that can be authenticated, /// - all block hashes referenced by the transactions in the batch. @@ -74,9 +74,10 @@ impl ProposedBatch { /// - Unauthenticated note authentication can be delayed to the block kernel. /// - Another transaction in the batch creates an output note matching an unauthenticated /// input note, in which case inclusion in the chain does not need to be proven. - /// - The block header's block number must be greater or equal to the highest block number - /// referenced by any transaction. This is not verified explicitly, but will implicitly cause - /// an error during validating that each reference block of a transaction is in the chain MMR. + /// - The reference block of a batch must satisfy the following requirement: Its block number + /// must be greater or equal to the highest block number referenced by any transaction. This + /// is not verified explicitly, but will implicitly cause an error during the validation that + /// each reference block of a transaction is in the chain MMR. /// /// # Errors /// @@ -110,7 +111,7 @@ impl ProposedBatch { /// reference block. pub fn new( transactions: Vec>, - block_header: BlockHeader, + reference_block_header: BlockHeader, chain_mmr: ChainMmr, unauthenticated_note_proofs: BTreeMap, ) -> Result { @@ -131,27 +132,35 @@ impl ProposedBatch { // Verify block header and chain MMR match. // -------------------------------------------------------------------------------------------- - if chain_mmr.chain_length() != block_header.block_num() { + if chain_mmr.chain_length() != reference_block_header.block_num() { return Err(ProposedBatchError::InconsistentChainLength { - expected: block_header.block_num(), + expected: reference_block_header.block_num(), actual: chain_mmr.chain_length(), }); } let hashed_peaks = chain_mmr.peaks().hash_peaks(); - if hashed_peaks != block_header.chain_root() { + if hashed_peaks != reference_block_header.chain_root() { return Err(ProposedBatchError::InconsistentChainRoot { - expected: block_header.chain_root(), + expected: reference_block_header.chain_root(), actual: hashed_peaks, }); } // Verify all block references from the transactions are in the chain MMR, except for the - // batch's reference block. + // batch's reference block. Note that a block X is only added to the blockchain MMR by + // block X + 1. This is because block X cannot compute its own block commitment and + // thus cannot add itself to the chain. So, more generally, a parent block is added to the + // blockchain by its child block. + // The reference block of a batch may be the latest block in the chain and, as mentioned, + // block is not yet part of the blockchain MMR, so its inclusion cannot be proven. Since the + // inclusion cannot be proven in all cases, the batch kernel instead commits to this + // reference block's commitment as a public input, which means the block kernel will + // prove this block's inclusion when including this batch and verifying its ZK proof. // -------------------------------------------------------------------------------------------- for tx in transactions.iter() { - if block_header.block_num() != tx.block_num() + if reference_block_header.block_num() != tx.block_num() && !chain_mmr.contains_block(tx.block_num()) { return Err(ProposedBatchError::MissingTransactionBlockReference { @@ -202,11 +211,11 @@ impl ProposedBatch { let mut batch_expiration_block_num = BlockNumber::from(u32::MAX); for tx in transactions.iter() { - if tx.expiration_block_num() <= block_header.block_num() { + if tx.expiration_block_num() <= reference_block_header.block_num() { return Err(ProposedBatchError::ExpiredTransaction { transaction_id: tx.id(), transaction_expiration_num: tx.expiration_block_num(), - reference_block_num: block_header.block_num(), + reference_block_num: reference_block_header.block_num(), }); } @@ -245,7 +254,7 @@ impl ProposedBatch { transactions.iter().map(AsRef::as_ref), &unauthenticated_note_proofs, &chain_mmr, - &block_header, + &reference_block_header, )?; if input_notes.len() > MAX_INPUT_NOTES_PER_BATCH { @@ -267,7 +276,7 @@ impl ProposedBatch { Ok(Self { id, transactions, - block_header, + reference_block_header, chain_mmr, unauthenticated_note_proofs, account_updates, @@ -338,7 +347,7 @@ impl ProposedBatch { ) { ( self.transactions, - self.block_header, + self.reference_block_header, self.chain_mmr, self.unauthenticated_note_proofs, self.id, @@ -361,7 +370,7 @@ impl Serializable for ProposedBatch { .collect::>() .write_into(target); - self.block_header.write_into(target); + self.reference_block_header.write_into(target); self.chain_mmr.write_into(target); self.unauthenticated_note_proofs.write_into(target); } @@ -460,7 +469,7 @@ mod tests { .context("failed to deserialize proposed batch")?; assert_eq!(batch.transactions(), batch2.transactions()); - assert_eq!(batch.block_header, batch2.block_header); + assert_eq!(batch.reference_block_header, batch2.reference_block_header); assert_eq!(batch.chain_mmr, batch2.chain_mmr); assert_eq!(batch.unauthenticated_note_proofs, batch2.unauthenticated_note_proofs); assert_eq!(batch.id, batch2.id); From 9ec51ce4e3b71923b3ede4416565354ebaa91afb Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 08:20:37 +0100 Subject: [PATCH 05/10] chore: Mention chain MMR <-> chain root match requirement --- crates/miden-objects/src/batch/proposed_batch.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/miden-objects/src/batch/proposed_batch.rs b/crates/miden-objects/src/batch/proposed_batch.rs index a28a42cdc..76594b788 100644 --- a/crates/miden-objects/src/batch/proposed_batch.rs +++ b/crates/miden-objects/src/batch/proposed_batch.rs @@ -64,7 +64,8 @@ impl ProposedBatch { /// matches the account state before any transactions are executed and B's initial account /// state commitment matches the final account state commitment of A, then A must come before /// B. - /// - The chain MMR should contain all block headers + /// - The chain MMR's hashed peaks must match the reference block's `chain_root` and it must + /// contain all block headers: /// - that are referenced by note inclusion proofs in `unauthenticated_note_proofs`. /// - that are referenced by a transaction in the batch. /// - The `unauthenticated_note_proofs` should contain [`NoteInclusionProof`]s for any From d0f63c00de1dd6c74ea7d9953ff4178b9895a247 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 08:25:48 +0100 Subject: [PATCH 06/10] chore: Note chain MMR contains check --- .../miden-objects/src/batch/proposed_batch.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/miden-objects/src/batch/proposed_batch.rs b/crates/miden-objects/src/batch/proposed_batch.rs index 76594b788..b13c5a4bf 100644 --- a/crates/miden-objects/src/batch/proposed_batch.rs +++ b/crates/miden-objects/src/batch/proposed_batch.rs @@ -149,15 +149,21 @@ impl ProposedBatch { } // Verify all block references from the transactions are in the chain MMR, except for the - // batch's reference block. Note that a block X is only added to the blockchain MMR by - // block X + 1. This is because block X cannot compute its own block commitment and - // thus cannot add itself to the chain. So, more generally, a parent block is added to the - // blockchain by its child block. + // batch's reference block. + // + // Note that some block X is only added to the blockchain MMR by block X + 1. This is + // because block X cannot compute its own block commitment and thus cannot add itself to the + // chain. So, more generally, a parent block is added to the blockchain by its child block. + // // The reference block of a batch may be the latest block in the chain and, as mentioned, // block is not yet part of the blockchain MMR, so its inclusion cannot be proven. Since the // inclusion cannot be proven in all cases, the batch kernel instead commits to this - // reference block's commitment as a public input, which means the block kernel will - // prove this block's inclusion when including this batch and verifying its ZK proof. + // reference block's commitment as a public input, which means the block kernel will prove + // this block's inclusion when including this batch and verifying its ZK proof. + // + // Finally, note that we don't verify anything cryptographically here. We have previously + // verified that the batch reference block's chain root matches the hashed peaks of the + // `ChainMmr`, and so we only have to check if the chain MMR contains the block here. // -------------------------------------------------------------------------------------------- for tx in transactions.iter() { From 846d6304af2d62fa0557cef6c4bf4ab3457cdab9 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 08:36:55 +0100 Subject: [PATCH 07/10] feat: Align naming of `ProvenBatch::created_nullifiers` --- crates/miden-block-prover/src/tests/proven_block_error.rs | 2 +- crates/miden-objects/src/batch/proven_batch.rs | 4 ++-- crates/miden-tx/src/testing/mock_chain/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/miden-block-prover/src/tests/proven_block_error.rs b/crates/miden-block-prover/src/tests/proven_block_error.rs index 55f90c32f..ae03ef44a 100644 --- a/crates/miden-block-prover/src/tests/proven_block_error.rs +++ b/crates/miden-block-prover/src/tests/proven_block_error.rs @@ -213,7 +213,7 @@ fn proven_block_fails_on_nullifier_tree_root_mismatch() -> anyhow::Result<()> { // Make the block inputs invalid by using a single stale nullifier witnesses. let mut invalid_nullifier_witness_block_inputs = valid_block_inputs.clone(); - let batch_nullifier0 = batches[0].produced_nullifiers().next().unwrap(); + let batch_nullifier0 = batches[0].created_nullifiers().next().unwrap(); core::mem::swap( invalid_nullifier_witness_block_inputs .nullifier_witnesses_mut() diff --git a/crates/miden-objects/src/batch/proven_batch.rs b/crates/miden-objects/src/batch/proven_batch.rs index bf9f59a27..9dc62b8d9 100644 --- a/crates/miden-objects/src/batch/proven_batch.rs +++ b/crates/miden-objects/src/batch/proven_batch.rs @@ -94,8 +94,8 @@ impl ProvenBatch { &self.input_notes } - /// Returns an iterator over the nullifiers produced in this batch. - pub fn produced_nullifiers(&self) -> impl Iterator + use<'_> { + /// Returns an iterator over the nullifiers created in this batch. + pub fn created_nullifiers(&self) -> impl Iterator + use<'_> { self.input_notes.iter().map(InputNoteCommitment::nullifier) } diff --git a/crates/miden-tx/src/testing/mock_chain/mod.rs b/crates/miden-tx/src/testing/mock_chain/mod.rs index 24a656f34..9e4a44282 100644 --- a/crates/miden-tx/src/testing/mock_chain/mod.rs +++ b/crates/miden-tx/src/testing/mock_chain/mod.rs @@ -761,7 +761,7 @@ impl MockChain { self.account_witnesses(batch_iterator.clone().flat_map(ProvenBatch::updated_accounts)); let nullifier_proofs = - self.nullifier_witnesses(batch_iterator.flat_map(ProvenBatch::produced_nullifiers)); + self.nullifier_witnesses(batch_iterator.flat_map(ProvenBatch::created_nullifiers)); BlockInputs::new( block_reference_block, From ce76f0f097623d85576cd6fea9739df73134c9fc Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 08:39:04 +0100 Subject: [PATCH 08/10] chore: Align naming of `ProposedBlock::created_nullifiers` --- .../src/tests/proposed_block_success.rs | 14 +++++++------- .../src/tests/proven_block_success.rs | 10 +++++----- crates/miden-objects/src/block/proposed_block.rs | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/miden-block-prover/src/tests/proposed_block_success.rs b/crates/miden-block-prover/src/tests/proposed_block_success.rs index b30249592..4ec4ab39b 100644 --- a/crates/miden-block-prover/src/tests/proposed_block_success.rs +++ b/crates/miden-block-prover/src/tests/proposed_block_success.rs @@ -31,7 +31,7 @@ fn proposed_block_succeeds_with_empty_batches() -> anyhow::Result<()> { assert_eq!(block.affected_accounts().count(), 0); assert_eq!(block.output_note_batches().len(), 0); - assert_eq!(block.nullifiers().len(), 0); + assert_eq!(block.created_nullifiers().len(), 0); assert_eq!(block.batches().len(), 0); Ok(()) @@ -72,12 +72,12 @@ fn proposed_block_basic_success() -> anyhow::Result<()> { proven_tx1.account_update().final_state_hash() ); // Each tx consumes one note. - assert_eq!(proposed_block.nullifiers().len(), 2); + assert_eq!(proposed_block.created_nullifiers().len(), 2); assert!(proposed_block - .nullifiers() + .created_nullifiers() .contains_key(&proven_tx0.input_notes().get_note(0).nullifier())); assert!(proposed_block - .nullifiers() + .created_nullifiers() .contains_key(&proven_tx1.input_notes().get_note(0).nullifier())); // There are two batches in the block... @@ -208,9 +208,9 @@ fn proposed_block_authenticating_unauthenticated_notes() -> anyhow::Result<()> { // We expect both notes to have been authenticated and therefore should be part of the // nullifiers of this block. - assert_eq!(proposed_block.nullifiers().len(), 2); - assert!(proposed_block.nullifiers().contains_key(¬e0.nullifier())); - assert!(proposed_block.nullifiers().contains_key(¬e1.nullifier())); + assert_eq!(proposed_block.created_nullifiers().len(), 2); + assert!(proposed_block.created_nullifiers().contains_key(¬e0.nullifier())); + assert!(proposed_block.created_nullifiers().contains_key(¬e1.nullifier())); // There are two batches in the block... assert_eq!(proposed_block.output_note_batches().len(), 2); // ... but none of them create notes. diff --git a/crates/miden-block-prover/src/tests/proven_block_success.rs b/crates/miden-block-prover/src/tests/proven_block_success.rs index 7572df806..bd0cc108d 100644 --- a/crates/miden-block-prover/src/tests/proven_block_success.rs +++ b/crates/miden-block-prover/src/tests/proven_block_success.rs @@ -93,7 +93,7 @@ fn proven_block_success() -> anyhow::Result<()> { // -------------------------------------------------------------------------------------------- let mut expected_nullifier_tree = chain.nullifiers().clone(); - for nullifier in proposed_block.nullifiers().keys() { + for nullifier in proposed_block.created_nullifiers().keys() { expected_nullifier_tree.insert( nullifier.inner(), [Felt::from(proposed_block.block_num()), Felt::ZERO, Felt::ZERO, Felt::ZERO], @@ -266,10 +266,10 @@ fn proven_block_erasing_unauthenticated_notes() -> anyhow::Result<()> { // The output note should have been erased, so we expect only the nullifiers of note0, note2 and // note3 to be created. - assert_eq!(proposed_block.nullifiers().len(), 3); - assert!(proposed_block.nullifiers().contains_key(¬e0.nullifier())); - assert!(proposed_block.nullifiers().contains_key(¬e2.nullifier())); - assert!(proposed_block.nullifiers().contains_key(¬e3.nullifier())); + assert_eq!(proposed_block.created_nullifiers().len(), 3); + assert!(proposed_block.created_nullifiers().contains_key(¬e0.nullifier())); + assert!(proposed_block.created_nullifiers().contains_key(¬e2.nullifier())); + assert!(proposed_block.created_nullifiers().contains_key(¬e3.nullifier())); // There are two batches in the block. assert_eq!(proposed_block.output_note_batches().len(), 2); diff --git a/crates/miden-objects/src/block/proposed_block.rs b/crates/miden-objects/src/block/proposed_block.rs index 725bb5209..7e25481c4 100644 --- a/crates/miden-objects/src/block/proposed_block.rs +++ b/crates/miden-objects/src/block/proposed_block.rs @@ -277,7 +277,7 @@ impl ProposedBlock { } /// Returns the map of nullifiers to their proofs from the proposed block. - pub fn nullifiers(&self) -> &BTreeMap { + pub fn created_nullifiers(&self) -> &BTreeMap { &self.created_nullifiers } From 2a39441715bf7216ecbd0216bc70604fc70cc6d3 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 21 Feb 2025 10:15:38 +0100 Subject: [PATCH 09/10] chore: Remove duplicate code --- crates/miden-objects/src/batch/proposed_batch.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/miden-objects/src/batch/proposed_batch.rs b/crates/miden-objects/src/batch/proposed_batch.rs index b13c5a4bf..c9dddadbf 100644 --- a/crates/miden-objects/src/batch/proposed_batch.rs +++ b/crates/miden-objects/src/batch/proposed_batch.rs @@ -183,7 +183,6 @@ impl ProposedBatch { // Populate batch output notes and updated accounts. let mut account_updates = BTreeMap::::new(); - let mut batch_expiration_block_num = BlockNumber::from(u32::MAX); for tx in transactions.iter() { // Merge account updates so that state transitions A->B->C become A->C. match account_updates.entry(tx.account_id()) { @@ -202,10 +201,6 @@ impl ProposedBatch { })?; }, }; - - // The expiration block of the batch is the minimum of all transaction's expiration - // block. - batch_expiration_block_num = batch_expiration_block_num.min(tx.expiration_block_num()); } if account_updates.len() > MAX_ACCOUNTS_PER_BATCH { From b6d06c32f9ec8e9926a39a35fc7b129b8bbdc9be Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 24 Feb 2025 10:49:10 +0100 Subject: [PATCH 10/10] feat: Let `BlockNoteIndex` return error if indices out of bounds --- .../src/local_block_prover.rs | 3 ++- .../src/tests/proven_block_success.rs | 6 +++++- crates/miden-objects/src/block/note_tree.rs | 17 +++++++++-------- crates/miden-objects/src/block/proven_block.rs | 3 ++- crates/miden-tx/src/testing/mock_chain/mod.rs | 13 +++++++++++-- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/crates/miden-block-prover/src/local_block_prover.rs b/crates/miden-block-prover/src/local_block_prover.rs index 05cb15c8e..4cd0f2bfd 100644 --- a/crates/miden-block-prover/src/local_block_prover.rs +++ b/crates/miden-block-prover/src/local_block_prover.rs @@ -296,7 +296,8 @@ fn compute_block_note_tree(output_note_batches: &[OutputNoteBatch]) -> BlockNote // SAFETY: The proposed block contains at most the max allowed number of // batches and each batch is guaranteed to contain at most // the max allowed number of output notes. - BlockNoteIndex::new(batch_idx, *note_idx_in_batch), + BlockNoteIndex::new(batch_idx, *note_idx_in_batch) + .expect("max batches in block and max notes in batches should be enforced"), note.id(), *note.metadata(), ) diff --git a/crates/miden-block-prover/src/tests/proven_block_success.rs b/crates/miden-block-prover/src/tests/proven_block_success.rs index bd0cc108d..d19d1beb8 100644 --- a/crates/miden-block-prover/src/tests/proven_block_success.rs +++ b/crates/miden-block-prover/src/tests/proven_block_success.rs @@ -84,7 +84,11 @@ fn proven_block_success() -> anyhow::Result<()> { let expected_block_note_tree = BlockNoteTree::with_entries(batch0_iter.chain(batch1_iter).map( |(batch_idx, note_idx_in_batch, note)| { - (BlockNoteIndex::new(batch_idx, note_idx_in_batch), note.id(), *note.metadata()) + ( + BlockNoteIndex::new(batch_idx, note_idx_in_batch).unwrap(), + note.id(), + *note.metadata(), + ) }, )) .unwrap(); diff --git a/crates/miden-objects/src/block/note_tree.rs b/crates/miden-objects/src/block/note_tree.rs index 906d76046..59c139b20 100644 --- a/crates/miden-objects/src/block/note_tree.rs +++ b/crates/miden-objects/src/block/note_tree.rs @@ -108,15 +108,16 @@ pub struct BlockNoteIndex { impl BlockNoteIndex { /// Creates a new [BlockNoteIndex]. /// - /// # Panics + /// # Errors /// - /// Panics if the batch index is equal to or greater than [`MAX_BATCHES_PER_BLOCK`] or if the - /// note index is equal to or greater than [`MAX_OUTPUT_NOTES_PER_BATCH`]. - pub fn new(batch_idx: usize, note_idx_in_batch: usize) -> Self { - assert!(note_idx_in_batch < MAX_OUTPUT_NOTES_PER_BATCH); - assert!(batch_idx < MAX_BATCHES_PER_BLOCK); - - Self { batch_idx, note_idx_in_batch } + /// Returns `None` if the batch index is equal to or greater than [`MAX_BATCHES_PER_BLOCK`] or + /// if the note index is equal to or greater than [`MAX_OUTPUT_NOTES_PER_BATCH`]. + pub fn new(batch_idx: usize, note_idx_in_batch: usize) -> Option { + if batch_idx >= MAX_BATCHES_PER_BLOCK || note_idx_in_batch >= MAX_OUTPUT_NOTES_PER_BATCH { + return None; + } + + Some(Self { batch_idx, note_idx_in_batch }) } /// Returns the batch index. diff --git a/crates/miden-objects/src/block/proven_block.rs b/crates/miden-objects/src/block/proven_block.rs index 1d47521e2..948d8e150 100644 --- a/crates/miden-objects/src/block/proven_block.rs +++ b/crates/miden-objects/src/block/proven_block.rs @@ -96,7 +96,8 @@ impl ProvenBlock { // SAFETY: The proven block contains at most the max allowed number of batches // and each batch is guaranteed to contain at most the // max allowed number of output notes. - BlockNoteIndex::new(batch_idx, *note_idx_in_batch), + BlockNoteIndex::new(batch_idx, *note_idx_in_batch) + .expect("max batches in block and max notes in batches should be enforced"), note, ) }) diff --git a/crates/miden-tx/src/testing/mock_chain/mod.rs b/crates/miden-tx/src/testing/mock_chain/mod.rs index 9e4a44282..42aa84945 100644 --- a/crates/miden-tx/src/testing/mock_chain/mod.rs +++ b/crates/miden-tx/src/testing/mock_chain/mod.rs @@ -185,7 +185,13 @@ impl PendingObjects { let entries = self.output_note_batches.iter().enumerate().flat_map(|(batch_index, batch)| { batch.iter().map(move |(note_index, note)| { - (BlockNoteIndex::new(batch_index, *note_index), note.id(), *note.metadata()) + ( + BlockNoteIndex::new(batch_index, *note_index).expect( + "max batches in block and max notes in batches should be enforced", + ), + note.id(), + *note.metadata(), + ) }) }); @@ -865,7 +871,10 @@ impl MockChain { for (note_index, note) in note_batch.iter() { match note { OutputNote::Full(note) => { - let block_note_index = BlockNoteIndex::new(batch_index, *note_index); + let block_note_index = BlockNoteIndex::new(batch_index, *note_index) + .expect( + "max batches in block and max notes in batches should be enforced", + ); let note_path = notes_tree.get_note_path(block_note_index); let note_inclusion_proof = NoteInclusionProof::new( block.header().block_num(),