resolved comments

CHANGELOG.md

@@ -24,6 +24,7 @@
 - Implemented `to_hex` for `AccountIdPrefix` and `epoch_block_num` for `BlockHeader` (#1039).
 - Introduce `AccountIdBuilder` to simplify `AccountId` generation in tests (#1045).
 - Introduced `AccountComponentTemplate` with TOML serialization and templating (#1015, #1027).
+- Add BlockNumber struct (feat: added BlockNumber struct #1043).
 
 ## 0.6.2 (2024-11-20)
 

miden-lib/src/transaction/inputs.rs

@@ -267,7 +267,7 @@ fn add_input_notes_to_advice_inputs(
                 } else {
                     tx_inputs
                         .block_chain()
-                        .get_block(block_num.as_u32())
+                        .get_block(block_num)
                         .expect("block not found in chain MMR")
                 };
 

miden-tx/src/testing/mock_chain/mod.rs

@@ -605,7 +605,7 @@ impl MockChain {
             let epoch_block_num = BlockNumber::from_epoch(account.id().anchor_epoch());
             // The reference block of the transaction is added to the MMR in
             // prologue::process_chain_data so we can skip adding it to the block headers here.
-            if epoch_block_num != block.header().block_num().into() {
+            if epoch_block_num != block.header().block_num() {
                 block_headers_map.insert(
                     epoch_block_num.as_u32(),
                     self.blocks.get(epoch_block_num.as_u32() as usize).unwrap().header(),

objects/src/accounts/account_id.rs

@@ -308,7 +308,7 @@ impl From<AccountIdPrefix> for AccountIdVersion {
 ///       hashes to the user's ID can claim the assets sent to the user's ID. Adding the anchor
 ///       block hash to ID generation process makes this attack practically impossible.
 ///
-/// [epoch_len_exp]: crate::block::BlockHeader::EPOCH_LENGTH_EXPONENT
+/// [epoch_len_exp]: crate::block::BlockNumber::EPOCH_LENGTH_EXPONENT
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub struct AccountId {
     first_felt: Felt,

objects/src/accounts/account_id_anchor.rs

@@ -12,8 +12,8 @@ use crate::{block::BlockNumber, AccountError, BlockHeader, Digest, EMPTY_WORD};
 /// # Constraints
 ///
 /// This type enforces the following constraints.
-/// - The `anchor_block_number` % 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`] must be zero. In other
-///   words, the block number must a multiple of 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`].
+/// - The `anchor_block_number` % 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`] must be zero. In other
+///   words, the block number must a multiple of 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`].
 /// - The epoch derived from the `anchor_block_number` must be strictly less than [`u16::MAX`].
 #[derive(Debug, Clone, Copy)]
 pub struct AccountIdAnchor {
@@ -49,9 +49,10 @@ impl AccountIdAnchor {
     /// Returns an error if any of the anchor constraints are not met. See the [type
     /// documentation](AccountIdAnchor) for details.
     pub fn new(
-        anchor_block_number: BlockNumber,
+        anchor_block_number: impl Into<BlockNumber>,
         anchor_block_hash: Digest,
     ) -> Result<Self, AccountError> {
+        let anchor_block_number = anchor_block_number.into();
         if anchor_block_number.as_u32() & 0x0000_ffff != 0 {
             return Err(AccountError::AssumptionViolated(format!(
           "TODO: Make proper error: anchor block must be an epoch block, i.e. its block number must be a multiple of 2^{}",
@@ -122,6 +123,6 @@ impl TryFrom<&BlockHeader> for AccountIdAnchor {
     /// Returns an error if any of the anchor constraints are not met. See the [type
     /// documentation](AccountIdAnchor) for details.
     fn try_from(block_header: &BlockHeader) -> Result<Self, Self::Error> {
-        Self::new(BlockNumber::from(block_header.block_num()), block_header.hash())
+        Self::new(block_header.block_num(), block_header.hash())
     }
 }

objects/src/accounts/builder/mod.rs

@@ -326,8 +326,7 @@ mod tests {
 
         let anchor_block_hash = Digest::new([Felt::new(42); 4]);
         let anchor_block_number = 1 << 16;
-        let id_anchor =
-            AccountIdAnchor::new(anchor_block_number.into(), anchor_block_hash).unwrap();
+        let id_anchor = AccountIdAnchor::new(anchor_block_number, anchor_block_hash).unwrap();
 
         let (account, seed) = Account::builder()
             .init_seed([5; 32])

objects/src/block/header.rs

@@ -44,9 +44,6 @@ pub struct BlockHeader {
 }
 
 impl BlockHeader {
-    /// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the
-    /// number of blocks in an epoch.
-
     /// Creates a new block header.
     #[allow(clippy::too_many_arguments)]
     pub fn new(
@@ -132,7 +129,7 @@ impl BlockHeader {
 
     /// Returns the epoch to which this block belongs.
     ///
-    /// This is the block number shifted right by [`Self::EPOCH_LENGTH_EXPONENT`].
+    /// This is the block number shifted right by [`BlockNumber::EPOCH_LENGTH_EXPONENT`].
     pub fn block_epoch(&self) -> u16 {
         self.block_num.block_epoch()
     }
@@ -274,12 +271,10 @@ impl Deserializable for BlockHeader {
 
 /// BLOCK NUMBER
 
-/// Holds `u32` type to signify Block Number
-
-#[derive(Debug, Eq, PartialEq, Copy, Clone, PartialOrd, Ord, Hash)]
 /// A convenience wrapper around a `u32` representing the number of a block.
 ///
 /// Each block has a unique number and block numbers increase monotonically by `1`.
+#[derive(Debug, Eq, PartialEq, Copy, Clone, PartialOrd, Ord, Hash)]
 pub struct BlockNumber(u32);
 
 impl Serializable for BlockNumber {
@@ -305,18 +300,24 @@ impl fmt::Display for BlockNumber {
 }
 
 impl BlockNumber {
+    /// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the
+    /// number of blocks in an epoch.
+    ///
     /// The epoch of a block can be obtained by shifting the block number to the right by this
     /// exponent.
     pub const EPOCH_LENGTH_EXPONENT: u8 = 16;
 
+    /// Creates the [`BlockNumber`] corresponding to the epoch block for the provided `epoch`.
     pub const fn from_epoch(epoch: u16) -> BlockNumber {
         BlockNumber((epoch as u32) << BlockNumber::EPOCH_LENGTH_EXPONENT)
     }
 
+    /// Returns the epoch to which this block number belongs.
     pub const fn block_epoch(&self) -> u16 {
         (self.0 >> BlockNumber::EPOCH_LENGTH_EXPONENT) as u16
     }
 
+    /// Returns the block number as a `u32`.
     pub fn as_u32(&self) -> u32 {
         self.0
     }

objects/src/errors.rs

@@ -279,16 +279,19 @@ pub enum ChainMmrError {
 }
 
 impl ChainMmrError {
-    pub fn block_num_too_big(chain_length: usize, block_num: u32) -> Self {
-        Self::BlockNumTooBig { chain_length, block_num }
+    pub fn block_num_too_big(chain_length: usize, block_num: BlockNumber) -> Self {
+        Self::BlockNumTooBig {
+            chain_length,
+            block_num: block_num.as_u32(),
+        }
     }
 
-    pub fn duplicate_block(block_num: u32) -> Self {
-        Self::DuplicateBlock { block_num }
+    pub fn duplicate_block(block_num: BlockNumber) -> Self {
+        Self::DuplicateBlock { block_num: block_num.as_u32() }
     }
 
-    pub fn untracked_block(block_num: u32) -> Self {
-        Self::UntrackedBlock { block_num }
+    pub fn untracked_block(block_num: BlockNumber) -> Self {
+        Self::UntrackedBlock { block_num: block_num.as_u32() }
     }
 }
 

objects/src/notes/location.rs

@@ -10,7 +10,7 @@ use crate::{
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct NoteLocation {
     /// The block number the note was created in.
-    block_num: u32,
+    block_num: BlockNumber,
 
     /// The index of the note in the note Merkle tree of the block the note was created in.
     node_index_in_block: u16,
@@ -19,7 +19,7 @@ pub struct NoteLocation {
 impl NoteLocation {
     /// Returns the block number the note was created in.
     pub fn block_num(&self) -> BlockNumber {
-        self.block_num.into()
+        self.block_num
     }
 
     /// Returns the index of the note in the note Merkle tree of the block the note was created in.
@@ -57,7 +57,10 @@ impl NoteInclusionProof {
                 highest_index: HIGHEST_INDEX,
             });
         }
-        let location = NoteLocation { block_num, node_index_in_block };
+        let location = NoteLocation {
+            block_num: block_num.into(),
+            node_index_in_block,
+        };
 
         Ok(Self { location, note_path })
     }
@@ -82,7 +85,7 @@ impl NoteInclusionProof {
 
 impl Serializable for NoteLocation {
     fn write_into<W: ByteWriter>(&self, target: &mut W) {
-        target.write_u32(self.block_num);
+        target.write_u32(self.block_num.as_u32());
         target.write_u16(self.node_index_in_block);
     }
 }
@@ -92,7 +95,10 @@ impl Deserializable for NoteLocation {
         let block_num = source.read_u32()?;
         let node_index_in_block = source.read_u16()?;
 
-        Ok(Self { block_num, node_index_in_block })
+        Ok(Self {
+            block_num: block_num.into(),
+            node_index_in_block,
+        })
     }
 }
 

objects/src/transaction/chain_mmr.rs

@@ -48,18 +48,15 @@ impl ChainMmr {
         let mut block_map = BTreeMap::new();
         for block in blocks.into_iter() {
             if block.block_num().as_u32() as usize >= chain_length {
-                return Err(ChainMmrError::block_num_too_big(
-                    chain_length,
-                    block.block_num().as_u32(),
-                ));
+                return Err(ChainMmrError::block_num_too_big(chain_length, block.block_num()));
             }
 
             if block_map.insert(block.block_num(), block).is_some() {
-                return Err(ChainMmrError::duplicate_block(block.block_num().as_u32()));
+                return Err(ChainMmrError::duplicate_block(block.block_num()));
             }
 
             if !mmr.is_tracked(block.block_num().as_u32() as usize) {
-                return Err(ChainMmrError::untracked_block(block.block_num().as_u32()));
+                return Err(ChainMmrError::untracked_block(block.block_num()));
             }
         }
 
@@ -80,14 +77,14 @@ impl ChainMmr {
     }
 
     /// Returns true if the block is present in this chain MMR.
-    pub fn contains_block(&self, block_num: u32) -> bool {
-        self.blocks.contains_key(&block_num.into())
+    pub fn contains_block(&self, block_num: BlockNumber) -> bool {
+        self.blocks.contains_key(&block_num)
     }
 
     /// Returns the block header for the specified block, or None if the block is not present in
     /// this chain MMR.
-    pub fn get_block(&self, block_num: u32) -> Option<&BlockHeader> {
-        self.blocks.get(&block_num.into())
+    pub fn get_block(&self, block_num: BlockNumber) -> Option<&BlockHeader> {
+        self.blocks.get(&block_num)
     }
 
     // DATA MUTATORS
@@ -155,6 +152,7 @@ mod tests {
     use super::ChainMmr;
     use crate::{
         alloc::vec::Vec,
+        block::BlockNumber,
         crypto::merkle::{Mmr, PartialMmr},
         BlockHeader, Digest,
     };
@@ -164,15 +162,15 @@ mod tests {
         // create chain MMR with 3 blocks - i.e., 2 peaks
         let mut mmr = Mmr::default();
         for i in 0..3 {
-            let block_header = int_to_block_header(i);
+            let block_header = int_to_block_header(i.into());
             mmr.add(block_header.hash());
         }
         let partial_mmr: PartialMmr = mmr.peaks().into();
         let mut chain_mmr = ChainMmr::new(partial_mmr, Vec::new()).unwrap();
 
         // add a new block to the chain MMR, this reduces the number of peaks to 1
         let block_num = 3;
-        let bock_header = int_to_block_header(block_num);
+        let bock_header = int_to_block_header(block_num.into());
         mmr.add(bock_header.hash());
         chain_mmr.add_block(bock_header, true);
 
@@ -183,7 +181,7 @@ mod tests {
 
         // add one more block to the chain MMR, the number of peaks is again 2
         let block_num = 4;
-        let bock_header = int_to_block_header(block_num);
+        let bock_header = int_to_block_header(block_num.into());
         mmr.add(bock_header.hash());
         chain_mmr.add_block(bock_header, true);
 
@@ -194,7 +192,7 @@ mod tests {
 
         // add one more block to the chain MMR, the number of peaks is still 2
         let block_num = 5;
-        let bock_header = int_to_block_header(block_num);
+        let bock_header = int_to_block_header(block_num.into());
         mmr.add(bock_header.hash());
         chain_mmr.add_block(bock_header, true);
 
@@ -209,7 +207,7 @@ mod tests {
         // create chain MMR with 3 blocks - i.e., 2 peaks
         let mut mmr = Mmr::default();
         for i in 0..3 {
-            let block_header = int_to_block_header(i);
+            let block_header = int_to_block_header(i.into());
             mmr.add(block_header.hash());
         }
         let partial_mmr: PartialMmr = mmr.peaks().into();
@@ -221,11 +219,11 @@ mod tests {
         assert_eq!(chain_mmr, deserialized);
     }
 
-    fn int_to_block_header(block_num: u32) -> BlockHeader {
+    fn int_to_block_header(block_num: BlockNumber) -> BlockHeader {
         BlockHeader::new(
             0,
             Digest::default(),
-            block_num,
+            block_num.as_u32(),
             Digest::default(),
             Digest::default(),
             Digest::default(),

objects/src/transaction/inputs.rs

@@ -67,7 +67,7 @@ impl TransactionInputs {
                     &block_header
                 } else {
                     block_chain
-                        .get_block(note_block_num.as_u32())
+                        .get_block(note_block_num)
                         .ok_or(TransactionInputError::InputNoteBlockNotInChainMmr(note.id()))?
                 };
 
@@ -486,7 +486,7 @@ pub fn validate_account_seed(
                 block_header.hash()
             } else {
                 let anchor_block_header =
-                    block_chain.get_block(anchor_block_number.as_u32()).ok_or_else(|| {
+                    block_chain.get_block(anchor_block_number).ok_or_else(|| {
                         TransactionInputError::AnchorBlockHeaderNotProvidedForNewAccount(
                             account.id().anchor_epoch(),
                         )