From 76a6635593ca168eda4517061ee0b4b845ad1c0f Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Mon, 16 Sep 2024 20:57:16 +0500 Subject: [PATCH] Refactored batch/block note tree structure (#834) * refactor: reduce note tree depths, don't store note's id/metadata in note tree * docs: update CHANGELOG.md * refactor: small code improvement * format: reformat using rustfmt * fix: return back `NOTE_LEAF_DEPTH` constant * refactor: use `NoteId` instead of digest in `BlockNoteTree` constructor * refactor: address review comments * fix: move validation to constructor of `BlockNoteIndex` * feat: implement `leaf_index_value` method in `BlockNoteIndex` * fix: check in `BlockNoteIndex` * fix: `MAX_NOTES_PER_BATCH` calculation * refactor: make assert expression cleaner --- CHANGELOG.md | 1 + .../kernels/transaction/lib/constants.masm | 2 +- .../asm/kernels/transaction/lib/prologue.masm | 2 +- miden-tx/src/testing/mock_chain.rs | 25 +++---- objects/src/batches/note_tree.rs | 16 ++--- objects/src/block/mod.rs | 12 ++-- objects/src/block/note_tree.rs | 69 ++++++++++--------- objects/src/constants.rs | 26 ++----- objects/src/notes/location.rs | 14 ++-- objects/src/notes/mod.rs | 14 +--- 10 files changed, 80 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e0d0e11..903f554d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - [BREAKING] `AccountStub` structure was renamed to `AccountHeader` (#855). - [BREAKING] Kernel procedures now have to be invoked using `dynexec` instruction (#803). - Refactored `AccountStorage` from `Smt` to `sequential hash` (#846) +- [BREAKING] Refactored batch/block note trees (#834). ## 0.5.1 (2024-08-28) - `miden-objects` crate only diff --git a/miden-lib/asm/kernels/transaction/lib/constants.masm b/miden-lib/asm/kernels/transaction/lib/constants.masm index c830d5aef..3763ad5c9 100644 --- a/miden-lib/asm/kernels/transaction/lib/constants.masm +++ b/miden-lib/asm/kernels/transaction/lib/constants.masm @@ -17,7 +17,7 @@ const.MAX_INPUT_NOTES_PER_TX=1023 const.NOTE_MEM_SIZE=512 # The depth of the Merkle tree used to commit to notes produced in a block. -const.NOTE_TREE_DEPTH=20 +const.NOTE_TREE_DEPTH=16 # The maximum number of notes that can be created in a single transaction (2^12). const.MAX_OUTPUT_NOTES_PER_TX=4096 diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 022fa12bf..1921f3ae2 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -1073,7 +1073,7 @@ proc.process_input_notes_data # - Below the hasher state in the stack is the current note index. This number is used for two # purposes: # 1. Compute the input note's memory addresses, the index works as an offset. - # 2. Determine the loop condition. The loop below runs until all inpute notes have been + # 2. Determine the loop condition. The loop below runs until all input notes have been # processed. # - The num_notes is kept at position 13, because dup.13 is cheap. # - The [idx, num_notes] pair is kept in a word boundary, so that its word can be swapped with a diff --git a/miden-tx/src/testing/mock_chain.rs b/miden-tx/src/testing/mock_chain.rs index 40206b659..3669eb019 100644 --- a/miden-tx/src/testing/mock_chain.rs +++ b/miden-tx/src/testing/mock_chain.rs @@ -128,13 +128,13 @@ impl PendingObjects { /// The root of the tree is a commitment to all notes created in the block. The commitment /// is not for all fields of the [Note] struct, but only for note metadata + core fields of /// a note (i.e., vault, inputs, script, and serial number). - pub fn build_notes_tree(&self) -> BlockNoteTree { + pub fn build_note_tree(&self) -> BlockNoteTree { let entries = self.output_note_batches.iter().enumerate().flat_map(|(batch_index, batch)| { batch.iter().enumerate().map(move |(note_index, note)| { ( - BlockNoteIndex::new(batch_index, note_index), - note.id().into(), + BlockNoteIndex::new(batch_index, note_index).unwrap(), + note.id(), *note.metadata(), ) }) @@ -430,13 +430,9 @@ impl MockChain { let mut block_headers_map: BTreeMap = BTreeMap::new(); for note in notes { let input_note = self.available_notes.get(note).unwrap().clone(); - block_headers_map.insert( - input_note.location().unwrap().block_num(), - self.blocks - .get(input_note.location().unwrap().block_num() as usize) - .unwrap() - .header(), - ); + let note_block_num = input_note.location().unwrap().block_num(); + let block_header = self.blocks.get(note_block_num as usize).unwrap().header(); + block_headers_map.insert(note_block_num, block_header); input_notes.push(input_note); } @@ -497,7 +493,7 @@ impl MockChain { for nullifier in self.pending_objects.created_nullifiers.iter() { self.nullifiers.insert(nullifier.inner(), [block_num.into(), ZERO, ZERO, ZERO]); } - let notes_tree = self.pending_objects.build_notes_tree(); + let notes_tree = self.pending_objects.build_note_tree(); let version = 0; let previous = self.blocks.last(); @@ -546,11 +542,12 @@ impl MockChain { // All note details should be OutputNote::Full at this point match note { OutputNote::Full(note) => { - let block_note_index = BlockNoteIndex::new(batch_index, note_index); - let note_path = notes_tree.get_note_path(block_note_index).unwrap(); + let block_note_index = + BlockNoteIndex::new(batch_index, note_index).unwrap(); + let note_path = notes_tree.get_note_path(block_note_index); let note_inclusion_proof = NoteInclusionProof::new( block.header().block_num(), - block_note_index.to_absolute_index(), + block_note_index.leaf_index_value(), note_path, ) .unwrap(); diff --git a/objects/src/batches/note_tree.rs b/objects/src/batches/note_tree.rs index e20adb0ae..39bcba6c3 100644 --- a/objects/src/batches/note_tree.rs +++ b/objects/src/batches/note_tree.rs @@ -4,16 +4,16 @@ use miden_crypto::{ }; use crate::{ - notes::{NoteId, NoteMetadata}, - BATCH_NOTES_TREE_DEPTH, + notes::{compute_note_hash, NoteId, NoteMetadata}, + BATCH_NOTE_TREE_DEPTH, }; -/// Wrapper over [SimpleSmt] for batch note tree. +/// Wrapper over [SimpleSmt] for batch note tree. /// -/// Each note is stored as two adjacent leaves: odd leaf for id, even leaf for metadata hash. +/// Value of each leaf is computed as: `hash(note_id || note_metadata)`. #[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub struct BatchNoteTree(SimpleSmt); +pub struct BatchNoteTree(SimpleSmt); impl BatchNoteTree { /// Wrapper around [`SimpleSmt::with_contiguous_leaves`] which populates notes at contiguous @@ -25,11 +25,11 @@ impl BatchNoteTree { pub fn with_contiguous_leaves<'a>( entries: impl IntoIterator, ) -> Result { - let interleaved = entries + let leaves = entries .into_iter() - .flat_map(|(note_id, metadata)| [note_id.into(), metadata.into()]); + .map(|(note_id, metadata)| compute_note_hash(note_id, metadata).into()); - SimpleSmt::with_contiguous_leaves(interleaved).map(Self) + SimpleSmt::with_contiguous_leaves(leaves).map(Self) } /// Returns the root of the tree diff --git a/objects/src/block/mod.rs b/objects/src/block/mod.rs index 23f71727f..9ae137ec4 100644 --- a/objects/src/block/mod.rs +++ b/objects/src/block/mod.rs @@ -102,16 +102,20 @@ impl Block { pub fn notes(&self) -> impl Iterator { self.output_note_batches.iter().enumerate().flat_map(|(batch_idx, notes)| { notes.iter().enumerate().map(move |(note_idx_in_batch, note)| { - (BlockNoteIndex::new(batch_idx, note_idx_in_batch), note) + ( + BlockNoteIndex::new(batch_idx, note_idx_in_batch).expect( + "Something went wrong: block is invalid, but passed or skipped validation", + ), + note, + ) }) }) } /// Returns a note tree containing all notes created in this block. pub fn build_note_tree(&self) -> BlockNoteTree { - let entries = self - .notes() - .map(|(note_index, note)| (note_index, note.id().into(), *note.metadata())); + let entries = + self.notes().map(|(note_index, note)| (note_index, note.id(), *note.metadata())); BlockNoteTree::with_entries(entries) .expect("Something went wrong: block is invalid, but passed or skipped validation") diff --git a/objects/src/block/note_tree.rs b/objects/src/block/note_tree.rs index a6ca8dbfb..737b9e79c 100644 --- a/objects/src/block/note_tree.rs +++ b/objects/src/block/note_tree.rs @@ -6,19 +6,20 @@ use miden_crypto::{ }; use crate::{ - notes::NoteMetadata, + notes::{compute_note_hash, NoteId, NoteMetadata}, utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, - BLOCK_NOTES_TREE_DEPTH, MAX_NOTES_PER_BATCH, MAX_NOTES_PER_BLOCK, + BlockError, BLOCK_NOTE_TREE_DEPTH, MAX_BATCHES_PER_BLOCK, MAX_NOTES_PER_BATCH, + MAX_NOTES_PER_BLOCK, }; -/// Wrapper over [SimpleSmt] for notes tree. +/// Wrapper over [SimpleSmt] for notes tree. /// /// Each note is stored as two adjacent leaves: odd leaf for id, even leaf for metadata hash. /// ID's leaf index is calculated as [(batch_idx * MAX_NOTES_PER_BATCH + note_idx_in_batch) * 2]. /// Metadata hash leaf is stored the next after id leaf: [id_index + 1]. #[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub struct BlockNoteTree(SimpleSmt); +pub struct BlockNoteTree(SimpleSmt); impl BlockNoteTree { /// Returns a new [BlockNoteTree] instantiated with entries set as specified by the provided @@ -26,21 +27,21 @@ impl BlockNoteTree { /// /// Entry format: (note_index, note_id, note_metadata). /// + /// Value of each leaf is computed as: `hash(note_id || note_metadata)`. /// All leaves omitted from the entries list are set to [crate::EMPTY_WORD]. /// /// # Errors /// Returns an error if: - /// - The number of entries exceeds the maximum notes tree capacity, that is 2^21. + /// - The number of entries exceeds the maximum notes tree capacity, that is 2^16. /// - The provided entries contain multiple values for the same key. pub fn with_entries( - entries: impl IntoIterator, + entries: impl IntoIterator, ) -> Result { - let interleaved = entries.into_iter().flat_map(|(index, note_id, metadata)| { - let id_index = index.leaf_index().into(); - [(id_index, note_id.into()), (id_index + 1, metadata.into())] + let leaves = entries.into_iter().map(|(index, note_id, metadata)| { + (index.leaf_index_value() as u64, compute_note_hash(note_id, &metadata).into()) }); - SimpleSmt::with_leaves(interleaved).map(Self) + SimpleSmt::with_leaves(leaves).map(Self) } /// Returns the root of the tree @@ -49,17 +50,9 @@ impl BlockNoteTree { } /// Returns merkle path for the note with specified batch/note indexes. - /// - /// The returned path is to the node which is the parent of both note and note metadata node. - pub fn get_note_path(&self, index: BlockNoteIndex) -> Result { - // get the path to the leaf containing the note (path len = 21) - let leaf_index = LeafIndex::new(index.leaf_index().into())?; - - // move up the path by removing the first node, this path now points to the parent of the - // note path - let note_path = self.0.open(&leaf_index).path[1..].to_vec(); - - Ok(note_path.into()) + pub fn get_note_path(&self, index: BlockNoteIndex) -> MerklePath { + // get the path to the leaf containing the note (path len = 16) + self.0.open(&index.leaf_index()).path } } @@ -78,8 +71,15 @@ pub struct BlockNoteIndex { impl BlockNoteIndex { /// Creates a new [BlockNoteIndex]. - pub fn new(batch_idx: usize, note_idx_in_batch: usize) -> Self { - Self { batch_idx, note_idx_in_batch } + pub fn new(batch_idx: usize, note_idx_in_batch: usize) -> Result { + if note_idx_in_batch >= MAX_NOTES_PER_BATCH { + return Err(BlockError::TooManyNotesInBatch(note_idx_in_batch)); + } + if batch_idx >= MAX_BATCHES_PER_BLOCK { + return Err(BlockError::TooManyTransactionBatches(batch_idx)); + } + + Ok(Self { batch_idx, note_idx_in_batch }) } /// Returns the batch index. @@ -92,16 +92,23 @@ impl BlockNoteIndex { self.note_idx_in_batch } - /// Returns an index to the node which the parent of both the note and note metadata. - pub fn to_absolute_index(&self) -> u32 { - const _: () = assert!(MAX_NOTES_PER_BLOCK <= u32::MAX as usize); - (self.batch_idx() * MAX_NOTES_PER_BATCH + self.note_idx_in_batch()) as u32 + /// Returns the leaf index of the note in the note tree. + pub fn leaf_index(&self) -> LeafIndex { + LeafIndex::new((self.batch_idx() * MAX_NOTES_PER_BATCH + self.note_idx_in_batch()) as u64) + .expect("Unreachable: Input values must be valid at this point") } - /// Returns an index of the leaf containing the note. - fn leaf_index(&self) -> u32 { - const _: () = assert!(MAX_NOTES_PER_BLOCK * 2 <= u32::MAX as usize); - self.to_absolute_index() * 2 + /// Returns the leaf index value of the note in the note tree. + pub fn leaf_index_value(&self) -> u16 { + const _: () = assert!( + MAX_NOTES_PER_BLOCK <= u16::MAX as usize + 1, + "Any note index is expected to fit in `u16`" + ); + + self.leaf_index() + .value() + .try_into() + .expect("Unreachable: Input values must be valid at this point") } } diff --git a/objects/src/constants.rs b/objects/src/constants.rs index be8768c6b..b90311c5f 100644 --- a/objects/src/constants.rs +++ b/objects/src/constants.rs @@ -1,9 +1,6 @@ /// Depth of the account database tree. pub const ACCOUNT_TREE_DEPTH: u8 = 64; -/// The depth of the Merkle tree used to commit to notes produced in a block. -pub const NOTE_TREE_DEPTH: u8 = 20; - /// The maximum number of assets that can be stored in a single note. pub const MAX_ASSETS_PER_NOTE: usize = 256; @@ -26,34 +23,19 @@ pub const MIN_PROOF_SECURITY_LEVEL: u32 = 96; // ================================================================================================ /// The depth of the Sparse Merkle Tree used to store output notes in a single batch. -/// -/// A single note uses two leaves in the tree. The even leaf is used to store the note's id, the -/// odd leaf is used to store the note's metadata. -pub const BATCH_NOTES_TREE_DEPTH: u8 = 13; +pub const BATCH_NOTE_TREE_DEPTH: u8 = 10; /// The maximum number of notes that can be created in a single batch. -/// -/// Because the tree used in a batch has fixed depth, and each note takes two leaves, the maximum -/// number of notes is the number of leaves in the tree. -pub const MAX_NOTES_PER_BATCH: usize = 2_usize.pow((BATCH_NOTES_TREE_DEPTH - 1) as u32); +pub const MAX_NOTES_PER_BATCH: usize = 1 << BATCH_NOTE_TREE_DEPTH; // BLOCK // ================================================================================================ -/// The depth of the Sparse Merkle Tree used to store a batch's note tree. -/// -/// This value can be interpreted as: -/// -/// - The depth of a tree with the leaves set to a batch output note tree root. -/// - The level at which the batches create note trees are merged, creating a new tree with this -/// many additional new levels. -pub const BLOCK_NOTES_BATCH_TREE_DEPTH: u8 = 8; - /// The final depth of the Sparse Merkle Tree used to store all notes created in a block. -pub const BLOCK_NOTES_TREE_DEPTH: u8 = BATCH_NOTES_TREE_DEPTH + BLOCK_NOTES_BATCH_TREE_DEPTH; +pub const BLOCK_NOTE_TREE_DEPTH: u8 = 16; /// Maximum number of batches that can be inserted into a single block. -pub const MAX_BATCHES_PER_BLOCK: usize = 2_usize.pow(BLOCK_NOTES_BATCH_TREE_DEPTH as u32); +pub const MAX_BATCHES_PER_BLOCK: usize = 1 << (BLOCK_NOTE_TREE_DEPTH - BATCH_NOTE_TREE_DEPTH); /// Maximum number of output notes that can be created in a single block. pub const MAX_NOTES_PER_BLOCK: usize = MAX_NOTES_PER_BATCH * MAX_BATCHES_PER_BLOCK; diff --git a/objects/src/notes/location.rs b/objects/src/notes/location.rs index a1a4732bd..de2951185 100644 --- a/objects/src/notes/location.rs +++ b/objects/src/notes/location.rs @@ -11,7 +11,7 @@ pub struct NoteLocation { block_num: u32, /// The index of the note in the note Merkle tree of the block the note was created in. - node_index_in_block: u32, + node_index_in_block: u16, } impl NoteLocation { @@ -24,9 +24,9 @@ impl NoteLocation { /// /// # Note /// - /// The height of the Merkle tree is [crate::constants::BLOCK_NOTES_TREE_DEPTH]. - /// Thus, the maximum index is `2 ^ BLOCK_NOTES_TREE_DEPTH - 1`. - pub fn node_index_in_block(&self) -> u32 { + /// The height of the Merkle tree is [crate::constants::BLOCK_NOTE_TREE_DEPTH]. + /// Thus, the maximum index is `2 ^ BLOCK_NOTE_TREE_DEPTH - 1`. + pub fn node_index_in_block(&self) -> u16 { self.node_index_in_block } } @@ -46,7 +46,7 @@ impl NoteInclusionProof { /// Returns a new [NoteInclusionProof]. pub fn new( block_num: u32, - node_index_in_block: u32, + node_index_in_block: u16, note_path: MerklePath, ) -> Result { const HIGHEST_INDEX: usize = MAX_BATCHES_PER_BLOCK * MAX_NOTES_PER_BATCH - 1; @@ -81,14 +81,14 @@ impl NoteInclusionProof { impl Serializable for NoteLocation { fn write_into(&self, target: &mut W) { target.write_u32(self.block_num); - target.write_u32(self.node_index_in_block); + target.write_u16(self.node_index_in_block); } } impl Deserializable for NoteLocation { fn read_from(source: &mut R) -> Result { let block_num = source.read_u32()?; - let node_index_in_block = source.read_u32()?; + let node_index_in_block = source.read_u16()?; Ok(Self { block_num, node_index_in_block }) } diff --git a/objects/src/notes/mod.rs b/objects/src/notes/mod.rs index 528be727a..601a08aac 100644 --- a/objects/src/notes/mod.rs +++ b/objects/src/notes/mod.rs @@ -6,10 +6,7 @@ use miden_crypto::{ }; use vm_processor::DeserializationError; -use crate::{ - accounts::AccountId, assets::Asset, Digest, Felt, Hasher, NoteError, NOTE_TREE_DEPTH, - WORD_SIZE, ZERO, -}; +use crate::{accounts::AccountId, assets::Asset, Digest, Felt, Hasher, NoteError, WORD_SIZE, ZERO}; mod assets; pub use assets::NoteAssets; @@ -56,15 +53,6 @@ pub use script::NoteScript; mod file; pub use file::NoteFile; -// CONSTANTS -// ================================================================================================ - -/// The depth of the leafs in the note Merkle tree used to commit to notes produced in a block. -/// -/// This is equal `NOTE_TREE_DEPTH + 1`. In the kernel we do not authenticate leaf data directly -/// but rather authenticate hash(left_leaf, right_leaf). -pub const NOTE_LEAF_DEPTH: u8 = NOTE_TREE_DEPTH + 1; - // NOTE // ================================================================================================