diff --git a/CHANGELOG.md b/CHANGELOG.md index 37970ebbc8..9c8bed29da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - [BREAKING] Implemented partial batch kernel verification to check the transaction list against `BATCH_ID` and compute `INPUT_NOTES_COMMITMENT`; `BatchExecutor::execute` now additionally takes caller `AdviceInputs` ([#2905](https://github.com/0xMiden/protocol/pull/2905)). +- [BREAKING] Constructed a `BatchNoteTree` over the batch's output notes during batch building: `ProposedBatch` exposes it via `batch_note_tree()` and includes it in `into_parts`, and `ProvenBatch` carries its root via `note_tree_root()` (changes the `ProvenBatch` serialization format and `new_unchecked` signature) ([#3022](https://github.com/0xMiden/protocol/pull/3022)). ### Changes diff --git a/crates/miden-protocol/src/batch/proposed_batch.rs b/crates/miden-protocol/src/batch/proposed_batch.rs index 9dafc68d67..0201b2835a 100644 --- a/crates/miden-protocol/src/batch/proposed_batch.rs +++ b/crates/miden-protocol/src/batch/proposed_batch.rs @@ -5,7 +5,7 @@ use alloc::vec::Vec; use crate::account::AccountId; use crate::batch::note_tracker::{NoteTracker, TrackerOutput}; -use crate::batch::{BatchAccountUpdate, BatchId}; +use crate::batch::{BatchAccountUpdate, BatchId, BatchNoteTree}; use crate::block::{BlockHeader, BlockNumber}; use crate::errors::ProposedBatchError; use crate::note::{NoteId, NoteInclusionProof}; @@ -63,6 +63,9 @@ pub struct ProposedBatch { /// batch that are not consumed within the same batch. These are sorted by /// [`OutputNote::id`]. output_notes: Vec, + /// The [`BatchNoteTree`] built over the batch's output notes, with note IDs packed at + /// contiguous leaf indices in the same order as `output_notes`. + batch_note_tree: BatchNoteTree, } impl ProposedBatch { @@ -320,6 +323,14 @@ impl ProposedBatch { return Err(ProposedBatchError::TooManyOutputNotes(output_notes.len())); } + // Build the batch note tree over the final output notes. `MAX_OUTPUT_NOTES_PER_BATCH` + // equals the tree's capacity (`2^BATCH_NOTE_TREE_DEPTH`) and the check above bounds the + // number of output notes by it, so this is a defensive error path that cannot be triggered + // in practice. + let batch_note_tree = + BatchNoteTree::with_contiguous_leaves(output_notes.iter().map(Into::into)) + .map_err(ProposedBatchError::BatchNoteTreeConstructionFailed)?; + // Compute batch ID. // -------------------------------------------------------------------------------------------- @@ -335,6 +346,7 @@ impl ProposedBatch { batch_expiration_block_num, input_notes, output_notes, + batch_note_tree, }) } @@ -452,6 +464,11 @@ impl ProposedBatch { &self.output_notes } + /// Returns the [`BatchNoteTree`] built over the batch's output notes. + pub fn batch_note_tree(&self) -> &BatchNoteTree { + &self.batch_note_tree + } + /// Consumes the proposed batch and returns its underlying parts. #[allow(clippy::type_complexity)] pub fn into_parts( @@ -466,6 +483,7 @@ impl ProposedBatch { InputNotes, Vec, BlockNumber, + BatchNoteTree, ) { ( self.transactions, @@ -477,6 +495,7 @@ impl ProposedBatch { self.input_notes, self.output_notes, self.batch_expiration_block_num, + self.batch_note_tree, ) } } @@ -613,6 +632,9 @@ mod tests { assert_eq!(batch.batch_expiration_block_num, batch2.batch_expiration_block_num); assert_eq!(batch.input_notes, batch2.input_notes); assert_eq!(batch.output_notes, batch2.output_notes); + // The batch note tree is not serialized but deterministically recomputed on + // deserialization. + assert_eq!(batch.batch_note_tree, batch2.batch_note_tree); Ok(()) } diff --git a/crates/miden-protocol/src/batch/proven_batch.rs b/crates/miden-protocol/src/batch/proven_batch.rs index c8af8c1cab..b9965a07c9 100644 --- a/crates/miden-protocol/src/batch/proven_batch.rs +++ b/crates/miden-protocol/src/batch/proven_batch.rs @@ -28,6 +28,13 @@ pub struct ProvenBatch { account_updates: BTreeMap, input_notes: InputNotes, output_notes: Vec, + /// The root of the [`BatchNoteTree`](crate::batch::BatchNoteTree) built over `output_notes`. + /// + /// This value is stored as-is and is **not** recomputed from or checked against `output_notes` + /// by [`ProvenBatch::new_unchecked`] or deserialization. It must therefore not be trusted at a + /// trust boundary until a consumer binds it to `output_notes` (e.g. the block kernel, which + /// will verify it against the per-batch note tree it reconstructs). + note_tree_root: Word, batch_expiration_block_num: BlockNumber, transactions: OrderedTransactionHeaders, proof: ExecutionProof, @@ -54,6 +61,7 @@ impl ProvenBatch { account_updates: BTreeMap, input_notes: InputNotes, output_notes: Vec, + note_tree_root: Word, batch_expiration_block_num: BlockNumber, transactions: OrderedTransactionHeaders, proof: ExecutionProof, @@ -73,6 +81,7 @@ impl ProvenBatch { account_updates, input_notes, output_notes, + note_tree_root, batch_expiration_block_num, transactions, proof, @@ -143,6 +152,16 @@ impl ProvenBatch { &self.output_notes } + /// Returns the root of the [`BatchNoteTree`](crate::batch::BatchNoteTree) built over the + /// batch's output notes. + /// + /// This root is not validated against [`Self::output_notes`] by the constructor or + /// deserialization, so it must not be trusted at a trust boundary until a consumer binds it to + /// the output notes. + pub fn note_tree_root(&self) -> Word { + self.note_tree_root + } + /// Returns the [`OrderedTransactionHeaders`] included in this batch. pub fn transactions(&self) -> &OrderedTransactionHeaders { &self.transactions @@ -172,6 +191,7 @@ impl Serializable for ProvenBatch { self.account_updates.write_into(target); self.input_notes.write_into(target); self.output_notes.write_into(target); + self.note_tree_root.write_into(target); self.batch_expiration_block_num.write_into(target); self.transactions.write_into(target); self.proof.write_into(target); @@ -185,6 +205,7 @@ impl Deserializable for ProvenBatch { let account_updates = BTreeMap::read_from(source)?; let input_notes = InputNotes::::read_from(source)?; let output_notes = Vec::::read_from(source)?; + let note_tree_root = Word::read_from(source)?; let batch_expiration_block_num = BlockNumber::read_from(source)?; let transactions = OrderedTransactionHeaders::read_from(source)?; let proof = ExecutionProof::read_from(source)?; @@ -199,6 +220,7 @@ impl Deserializable for ProvenBatch { account_updates, input_notes, output_notes, + note_tree_root, batch_expiration_block_num, transactions, proof, diff --git a/crates/miden-protocol/src/errors/mod.rs b/crates/miden-protocol/src/errors/mod.rs index 48a704138b..6529dd4d98 100644 --- a/crates/miden-protocol/src/errors/mod.rs +++ b/crates/miden-protocol/src/errors/mod.rs @@ -1074,6 +1074,9 @@ pub enum ProposedBatchError { )] TooManyOutputNotes(usize), + #[error("failed to construct the batch note tree from the batch's output notes")] + BatchNoteTreeConstructionFailed(#[source] MerkleError), + #[error( "transaction batch has {0} account updates but at most {MAX_ACCOUNTS_PER_BATCH} are allowed" )] diff --git a/crates/miden-testing/src/kernel_tests/batch/proposed_batch.rs b/crates/miden-testing/src/kernel_tests/batch/proposed_batch.rs index 6bfe1f30ac..fe1876a4b4 100644 --- a/crates/miden-testing/src/kernel_tests/batch/proposed_batch.rs +++ b/crates/miden-testing/src/kernel_tests/batch/proposed_batch.rs @@ -1,4 +1,5 @@ use alloc::sync::Arc; +use core::slice; use std::collections::BTreeMap; use anyhow::Context; @@ -7,7 +8,7 @@ use miden_crypto::rand::RandomCoin; use miden_protocol::Word; use miden_protocol::account::{Account, AccountId, AccountType}; use miden_protocol::asset::NonFungibleAsset; -use miden_protocol::batch::ProposedBatch; +use miden_protocol::batch::{BatchNoteTree, ProposedBatch, ProvenBatch}; use miden_protocol::block::BlockNumber; use miden_protocol::crypto::merkle::MerkleError; use miden_protocol::errors::{BatchAccountUpdateError, ProposedBatchError, ProvenBatchError}; @@ -29,6 +30,7 @@ use miden_protocol::transaction::{ ProvenTransaction, RawOutputNote, }; +use miden_protocol::utils::serde::{Deserializable, Serializable}; use miden_protocol::vm::AdviceInputs; use miden_standards::note::P2idNoteStorage; use miden_standards::testing::account_component::MockAccountComponent; @@ -236,6 +238,150 @@ fn note_created_and_consumed_in_same_batch() -> anyhow::Result<()> { Ok(()) } +/// Tests that the batch note tree is built over the batch's final output notes: its root matches a +/// tree built independently from `output_notes()` and it has one leaf per output note. +#[test] +fn batch_note_tree_built_over_output_notes() -> anyhow::Result<()> { + let TestSetup { mut chain, account1, .. } = setup_chain(); + let block1 = chain.block_header(1); + let block2 = chain.prove_next_block()?; + + let output_notes = (40..43).map(mock_output_note).collect::>(); + let tx = + MockProvenTxBuilder::with_account(account1.id(), Word::empty(), account1.to_commitment()) + .reference_block(&block1) + .output_notes(output_notes.clone()) + .build()?; + + let batch = ProposedBatch::new_unverified( + [tx].into_iter().map(Arc::new).collect(), + block2.header().clone(), + chain.latest_partial_blockchain(), + BTreeMap::default(), + )?; + + assert_eq!(batch.output_notes().len(), output_notes.len()); + + let expected_tree = + BatchNoteTree::with_contiguous_leaves(batch.output_notes().iter().map(Into::into))?; + assert_eq!(batch.batch_note_tree().root(), expected_tree.root()); + assert_eq!(batch.batch_note_tree().num_leaves(), output_notes.len()); + assert_ne!( + batch.batch_note_tree().root(), + BatchNoteTree::with_contiguous_leaves([])?.root() + ); + + Ok(()) +} + +/// Tests that notes erased within a batch (created and consumed in the same batch) are excluded +/// from the batch note tree, so its root matches a tree built from the post-erasure output notes. +#[test] +fn batch_note_tree_excludes_erased_notes() -> anyhow::Result<()> { + let TestSetup { mut chain, account1, account2, .. } = setup_chain(); + let block1 = chain.block_header(1); + let block2 = chain.prove_next_block()?; + + // tx1 creates an erased note (consumed by tx2) and a kept note. + let erased_note = mock_note(40); + let kept_note = mock_output_note(41); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Word::empty(), account1.to_commitment()) + .reference_block(&block1) + .output_notes(vec![ + RawOutputNote::Full(erased_note.clone()).into_output_note().unwrap(), + kept_note.clone(), + ]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Word::empty(), account2.to_commitment()) + .reference_block(&block1) + .unauthenticated_notes(vec![erased_note.clone()]) + .build()?; + + let batch = ProposedBatch::new_unverified( + [tx1, tx2].into_iter().map(Arc::new).collect(), + block2.header().clone(), + chain.latest_partial_blockchain(), + BTreeMap::default(), + )?; + + // Only the kept note survives erasure. + assert_eq!(batch.output_notes(), slice::from_ref(&kept_note)); + assert_eq!(batch.batch_note_tree().num_leaves(), 1); + + let expected_tree = + BatchNoteTree::with_contiguous_leaves(slice::from_ref(&kept_note).iter().map(Into::into))?; + assert_eq!(batch.batch_note_tree().root(), expected_tree.root()); + + Ok(()) +} + +/// Tests that a batch without output notes has an empty batch note tree, and that the root carried +/// on the proven batch matches the proposed batch's tree root. +#[test] +fn batch_note_tree_empty_when_no_output_notes() -> anyhow::Result<()> { + let TestSetup { mut chain, account1, .. } = setup_chain(); + let block1 = chain.block_header(1); + let block2 = chain.prove_next_block()?; + + let tx = + MockProvenTxBuilder::with_account(account1.id(), Word::empty(), account1.to_commitment()) + .reference_block(&block1) + .build()?; + + let proposed_batch = ProposedBatch::new_unverified( + [tx].into_iter().map(Arc::new).collect(), + block2.header().clone(), + chain.latest_partial_blockchain(), + BTreeMap::default(), + )?; + + assert_eq!(proposed_batch.output_notes().len(), 0); + assert_eq!(proposed_batch.batch_note_tree().num_leaves(), 0); + assert_eq!( + proposed_batch.batch_note_tree().root(), + BatchNoteTree::with_contiguous_leaves([])?.root(), + ); + + // The proven batch carries the same note tree root. + let proven_batch = chain.prove_transaction_batch(proposed_batch.clone())?; + assert_eq!(proven_batch.note_tree_root(), proposed_batch.batch_note_tree().root()); + + Ok(()) +} + +/// Tests that a proven batch's note tree root survives a serialization round-trip. +#[test] +fn proven_batch_serialization_preserves_note_tree_root() -> anyhow::Result<()> { + let TestSetup { mut chain, account1, .. } = setup_chain(); + let block1 = chain.block_header(1); + let block2 = chain.prove_next_block()?; + + let output_notes = (40..43).map(mock_output_note).collect::>(); + let tx = + MockProvenTxBuilder::with_account(account1.id(), Word::empty(), account1.to_commitment()) + .reference_block(&block1) + .output_notes(output_notes) + .build()?; + + let proposed_batch = ProposedBatch::new_unverified( + [tx].into_iter().map(Arc::new).collect(), + block2.header().clone(), + chain.latest_partial_blockchain(), + BTreeMap::default(), + )?; + let expected_root = proposed_batch.batch_note_tree().root(); + let proven_batch = chain.prove_transaction_batch(proposed_batch)?; + assert_eq!(proven_batch.note_tree_root(), expected_root); + + let deserialized = ProvenBatch::read_from_bytes(&proven_batch.to_bytes()).unwrap(); + assert_eq!(deserialized, proven_batch); + assert_eq!(deserialized.note_tree_root(), expected_root); + + Ok(()) +} + /// Notes with the same details but different metadata are not considered the same for batch /// erasure. #[test] diff --git a/crates/miden-tx-batch/src/local_batch_prover.rs b/crates/miden-tx-batch/src/local_batch_prover.rs index 8eec60af3b..de2a82cc38 100644 --- a/crates/miden-tx-batch/src/local_batch_prover.rs +++ b/crates/miden-tx-batch/src/local_batch_prover.rs @@ -70,6 +70,7 @@ impl LocalBatchProver { input_notes, output_notes, batch_expiration_block_num, + batch_note_tree, ) = proposed_batch.into_parts(); ProvenBatch::new_unchecked( @@ -79,6 +80,7 @@ impl LocalBatchProver { updated_accounts, input_notes, output_notes, + batch_note_tree.root(), batch_expiration_block_num, tx_headers, proof,